Skip to content

Removed prerelease flag from schema loading#1276

Merged
VisLab merged 4 commits intohed-standard:mainfrom
VisLab:fix_extras
Mar 20, 2026
Merged

Removed prerelease flag from schema loading#1276
VisLab merged 4 commits intohed-standard:mainfrom
VisLab:fix_extras

Conversation

@VisLab
Copy link
Member

@VisLab VisLab commented Mar 20, 2026

No description provided.

@github-actions
Copy link

PR Review: Remove check_prerelease flag from schema loading

The simplification goal is sound and the implementation is clean. One important backward-compat issue needs addressing before merge.

IMPORTANT ISSUES:

  1. Breaking public API change without deprecation path. load_schema_version, load_schema, from_string, and from_dataframes are all exported via hed/init.py. Removing check_prerelease entirely means any downstream code that passes check_prerelease=True or check_prerelease=False will get a TypeError after upgrading, with no hint about what changed. The recommended fix is to accept the old keyword arg for one release and emit a DeprecationWarning. See inline comment on hed_schema_io.py.

  2. CHANGELOG mislabels the severity. The removal is listed under Enhancements. Since it breaks existing callers, it warrants a Breaking Changes section. See inline comment on CHANGELOG.md.

SUGGESTION:

get_hed_versions default flip (False to True) is a quieter behavioral change. Existing callers relying on the old default now silently receive prerelease versions. The compliance checker correctly guards this with an explicit check_prerelease=False. Worth noting in the CHANGELOG for anyone calling hed_cache directly.

WHAT LOOKS GOOD:

  • Core logic is correct: regular path tried first, prerelease as fallback, no regression.
  • Compliance check explicitly passes check_prerelease=False where released-only semantics are required.
  • Test suite properly replaces old flag-conditional tests with direct behavior assertions.
  • _is_prerelease_partner helper cleanly removed after the flag is no longer needed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the check_prerelease opt-in flag across the schema loading surface area and makes prerelease schemas discoverable automatically during version/path lookups, updating tests, scripts, and documentation to match.

Changes:

  • Removed check_prerelease from schema-loading public APIs (load_schema_version, load_schema, from_string, from_dataframes) and schema loader classes.
  • Updated cache lookup behavior so prerelease directories are searched automatically (regular first), and adjusted compliance checks to explicitly compare against released-only versions.
  • Updated unit/spec tests, fixtures, and the changelog to reflect the new prerelease lookup behavior.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/schema/test_hed_schema_io.py Reworked tests to validate prerelease schemas load without an opt-in flag and partner resolution works.
tests/data/schema_tests/prerelease/HED_testpre_1.0.0.mediawiki Updated fixture prologue text to match new prerelease behavior.
spec_tests/test_hed_cache.py Updated spec test to rely on new default prerelease-inclusive version listing.
lychee.toml Removed a comment line (no functional impact).
hed/scripts/schema_script_util.py Removed prerelease-partner detection and stopped threading prerelease flags through roundtrip reloads.
hed/scripts/check_schema_loading.py Simplified loader checks by removing prerelease flag plumbing.
hed/schema/schema_validation/compliance.py Made compliance explicitly compare against released-only versions by passing check_prerelease=False.
hed/schema/schema_io/xml2schema.py Removed prerelease flag from XML loader constructor/super call.
hed/schema/schema_io/wiki2schema.py Removed prerelease flag from MediaWiki loader constructor/super call.
hed/schema/schema_io/json2schema.py Removed prerelease flag from JSON loader constructor/super call.
hed/schema/schema_io/df2schema.py Removed prerelease flag from dataframe/TSV loader APIs.
hed/schema/schema_io/base2schema.py Removed prerelease state from base loader and always allow partner resolution via normal version lookup.
hed/schema/hed_schema_io.py Removed prerelease flag threading from all public schema-loading functions and internal helpers.
hed/schema/hed_cache.py Defaulted version listing to include prereleases and made version-path lookup always check prerelease directory.
CHANGELOG.md Documented the API change and new prerelease lookup behavior.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Comment on lines +82 to +90
all_versions = hed_cache.get_hed_versions(self.hed_cache_dir, library_name="score")
released_only = hed_cache.get_hed_versions(self.hed_cache_dir, library_name="score", check_prerelease=False)
self.assertIsInstance(all_versions, list)
self.assertTrue(len(all_versions) > 0)
self.assertGreater(
len(all_versions),
len(released_only),
"Default (prerelease included) should return more versions than released-only",
)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion can be flaky because it relies on the remote hed-schemas repo having at least one prerelease version for the score library at test time. If there are no prerelease versions (or they get removed), len(all_versions) will equal len(released_only) and the test will fail even though the code is correct. Consider making the test deterministic by creating a prerelease xml file under {hed_cache_dir}/prerelease/ (or using a local fixture/mocking) and asserting that the default call includes it while check_prerelease=False does not.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

test

@VisLab VisLab merged commit e90e05b into hed-standard:main Mar 20, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants