Skip to content

Commit 48e1ab9

Browse files
committed
Addressed copilot review
1 parent 57056dd commit 48e1ab9

6 files changed

Lines changed: 302 additions & 13 deletions

File tree

hed/schema/hed_schema_io.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,15 @@ def from_string(
174174
return hed_schema
175175

176176

177-
def from_dataframes(schema_data, schema_namespace=None, name=None) -> "HedSchema":
177+
def from_dataframes(schema_data, schema_namespace=None, name=None, check_prerelease=False) -> "HedSchema":
178178
"""Create a schema from the given string.
179179
180180
Parameters:
181181
schema_data (dict of str or None): A dict of DF_SUFFIXES:file_as_string_or_df
182182
Should have an entry for all values of DF_SUFFIXES.
183183
schema_namespace (str, None): The name_prefix all tags in this schema will accept.
184184
name (str or None): User supplied identifier for this schema
185+
check_prerelease (bool): If True, allow the partnered standard schema (withStandard) to be a prerelease version.
185186
186187
Returns:
187188
HedSchema: The loaded schema.
@@ -199,7 +200,9 @@ def from_dataframes(schema_data, schema_namespace=None, name=None) -> "HedSchema
199200
HedExceptions.BAD_PARAMETERS, "Empty or non dict value passed to HedSchema.from_dataframes", filename=name
200201
)
201202

202-
hed_schema = SchemaLoaderDF.load_spreadsheet(schema_as_strings_or_df=schema_data, name=name)
203+
hed_schema = SchemaLoaderDF.load_spreadsheet(
204+
schema_as_strings_or_df=schema_data, name=name, check_prerelease=check_prerelease
205+
)
203206

204207
if schema_namespace:
205208
hed_schema.set_schema_prefix(schema_namespace=schema_namespace)

hed/scripts/check_schema_loading.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,13 +382,13 @@ def run_loading_check(
382382
and 'failures' (list of dicts with 'path' and 'error').
383383
384384
Raises:
385-
ValueError: If mutually exclusive flags are combined (e.g., prerelease_only and exclude_prereleases,
386-
or library_filter and standard_only).
385+
ValueError: If mutually exclusive flags are combined (e.g., --exclude-prereleases and
386+
--prerelease-only, or --library and --standard-only).
387387
"""
388388
if prerelease_only and exclude_prereleases:
389-
raise ValueError("prerelease_only and exclude_prereleases are mutually exclusive")
389+
raise ValueError("--exclude-prereleases and --prerelease-only are mutually exclusive")
390390
if library_filter and standard_only:
391-
raise ValueError("library_filter and standard_only are mutually exclusive")
391+
raise ValueError("--library and --standard-only are mutually exclusive")
392392

393393
tester = SchemaLoadTester(hed_schemas_root, verbose=verbose)
394394

hed/scripts/schema_script_util.py

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,37 @@
11
import os.path
22
from collections import defaultdict
33
from hed.schema import from_string, load_schema, from_dataframes
4+
from hed.schema import hed_cache
45
from hed.errors import get_printable_issue_string, HedFileError
56
from hed.errors.error_types import ErrorSeverity
67
from hed.schema.schema_comparer import SchemaComparer
78

89
all_extensions = [".tsv", ".mediawiki", ".xml", ".json"]
910

1011

12+
def _is_prerelease_partner(base_schema) -> bool:
13+
"""Return True if base_schema's withStandard partner is only resolvable from the prerelease cache.
14+
15+
When a library schema serialised with ``save_merged=False`` is reloaded, the loader
16+
re-fetches the standard schema named in the ``withStandard`` header attribute. If
17+
that version lives only in the prerelease subdirectory of the cache, the reload will
18+
fail unless ``check_prerelease=True`` is forwarded. This helper detects that
19+
condition by asking the cache whether the version is found without the prerelease
20+
flag (not found → prerelease required).
21+
22+
Parameters:
23+
base_schema (HedSchema): The schema to inspect.
24+
25+
Returns:
26+
bool: True if ``withStandard`` is set and the version is absent from the
27+
regular (non-prerelease) cache directory.
28+
"""
29+
with_standard = base_schema.with_standard
30+
if not with_standard:
31+
return False
32+
return hed_cache.get_hed_version_path(with_standard, check_prerelease=False) is None
33+
34+
1135
def validate_schema_object(base_schema, schema_name):
1236
"""Validate a schema object by checking compliance and roundtrip conversion.
1337
@@ -30,10 +54,16 @@ def validate_schema_object(base_schema, schema_name):
3054
validation_issues.append(error_message)
3155
return validation_issues
3256

57+
# If the withStandard partner only exists in the prerelease cache, all unmerged
58+
# reloads must pass check_prerelease=True or they will fail partner resolution.
59+
check_prerelease = _is_prerelease_partner(base_schema)
60+
3361
for save_merged in (True, False):
3462
label = "merged" if save_merged else "unmerged"
3563
tagged_name = f"{schema_name} ({label})"
36-
validation_issues += _roundtrip_all_formats(base_schema, tagged_name, save_merged=save_merged)
64+
validation_issues += _roundtrip_all_formats(
65+
base_schema, tagged_name, save_merged=save_merged, check_prerelease=check_prerelease
66+
)
3767
except HedFileError as e:
3868
print(f"Saving/loading error: {schema_name} {e.message}")
3969
error_text = e.message
@@ -291,7 +321,7 @@ def get_prerelease_path(repo_path, schema_name, schema_version):
291321
return os.path.join(base_path, "hedtsv", schema_filename)
292322

293323

294-
def _roundtrip_all_formats(base_schema, schema_name, save_merged=True):
324+
def _roundtrip_all_formats(base_schema, schema_name, save_merged=True, check_prerelease=False):
295325
"""Roundtrip a schema through all four formats and compare to the original.
296326
297327
Serializes the schema to mediawiki, XML, JSON, and TSV, reloads each, and
@@ -302,26 +332,32 @@ def _roundtrip_all_formats(base_schema, schema_name, save_merged=True):
302332
schema_name (str): Label for error reporting (should include merge context).
303333
save_merged (bool): If True, save the merged (with-standard) form.
304334
If False, save only the library-specific content.
335+
check_prerelease (bool): If True, pass check_prerelease=True to all reload
336+
calls. Required when the schema's withStandard partner exists only in
337+
the prerelease cache directory; otherwise unmerged reloads will fail
338+
partner resolution. Has no effect when save_merged=True because the
339+
merged serialisation embeds the full standard content and no partner
340+
lookup is performed on reload.
305341
306342
Returns:
307343
list: A list of validation issue strings. Empty if no issues found.
308344
"""
309345
issues = []
310346

311347
mediawiki_string = base_schema.get_as_mediawiki_string(save_merged=save_merged)
312-
reloaded_schema = from_string(mediawiki_string, schema_format=".mediawiki")
348+
reloaded_schema = from_string(mediawiki_string, schema_format=".mediawiki", check_prerelease=check_prerelease)
313349
issues += _get_schema_comparison(base_schema, reloaded_schema, schema_name, "mediawiki")
314350

315351
xml_string = base_schema.get_as_xml_string(save_merged=save_merged)
316-
reloaded_schema = from_string(xml_string, schema_format=".xml")
352+
reloaded_schema = from_string(xml_string, schema_format=".xml", check_prerelease=check_prerelease)
317353
issues += _get_schema_comparison(base_schema, reloaded_schema, schema_name, "xml")
318354

319355
json_string = base_schema.get_as_json_string(save_merged=save_merged)
320-
reloaded_schema = from_string(json_string, schema_format=".json")
356+
reloaded_schema = from_string(json_string, schema_format=".json", check_prerelease=check_prerelease)
321357
issues += _get_schema_comparison(base_schema, reloaded_schema, schema_name, "json")
322358

323359
tsv_dataframes = base_schema.get_as_dataframes(save_merged=save_merged)
324-
reloaded_schema = from_dataframes(tsv_dataframes)
360+
reloaded_schema = from_dataframes(tsv_dataframes, check_prerelease=check_prerelease)
325361
issues += _get_schema_comparison(base_schema, reloaded_schema, schema_name, "tsv")
326362

327363
return issues
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
HED version="9.9.9"
2+
3+
'''Prologue'''
4+
Minimal prerelease-only standard schema for unit testing withStandard partner
5+
resolution via load_schema/from_string.
6+
7+
!# start schema
8+
9+
'''Event''' <nowiki>[Something that happens at a given time and place.]</nowiki>
10+
11+
'''Property''' <nowiki>[A characteristic of something.]</nowiki>
12+
13+
!# end schema
14+
15+
'''Unit classes'''
16+
17+
'''Unit modifiers'''
18+
19+
'''Value classes'''
20+
21+
'''Schema attributes'''
22+
23+
'''Properties'''
24+
25+
'''Epilogue'''
26+
27+
!# end hed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
HED library="testpre" version="1.0.0" withStandard="9.9.9" unmerged="True"
2+
3+
'''Prologue'''
4+
Library schema whose withStandard partner (9.9.9) exists only in the prerelease
5+
directory. Used to test check_prerelease propagation through load_schema/from_string.
6+
7+
!# start schema
8+
9+
'''Prerelease-partner-only-item''' <nowiki>[A test item in the testpre library schema.]</nowiki>
10+
11+
!# end schema
12+
13+
'''Unit classes'''
14+
15+
'''Unit modifiers'''
16+
17+
'''Value classes'''
18+
19+
'''Schema attributes'''
20+
21+
'''Properties'''
22+
23+
'''Epilogue'''
24+
25+
!# end hed

0 commit comments

Comments
 (0)