fix: sort LicenseType members by value for reproducible schemas - #434
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #434 +/- ##
=======================================
Coverage 47.48% 47.48%
=======================================
Files 19 19
Lines 2367 2367
=======================================
Hits 1124 1124
Misses 1243 1243
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # The `sorted()` call is required, not cosmetic. `Config.licenses` is a `set`, | ||
| # and the order of iterating a `set` of `Enum` members depends on | ||
| # `PYTHONHASHSEED`, which CPython randomizes per process. Without sorting, the | ||
| # member order, and hence the `enum` array of every JSON schema generated from | ||
| # these models, varies from run to run. Pydantic emits member values rather | ||
| # than names into the `enum` array, which is why `value` is the sort key. |
There was a problem hiding this comment.
| # The `sorted()` call is required, not cosmetic. `Config.licenses` is a `set`, | |
| # and the order of iterating a `set` of `Enum` members depends on | |
| # `PYTHONHASHSEED`, which CPython randomizes per process. Without sorting, the | |
| # member order, and hence the `enum` array of every JSON schema generated from | |
| # these models, varies from run to run. Pydantic emits member values rather | |
| # than names into the `enum` array, which is why `value` is the sort key. | |
| # `sorted()` since `Config.licenses` is a `set`. |
is likely the absolute maximum needed here, the rest is too AIsloppy to my taste, or am I missing some detail which might be crucial if omitted?
There was a problem hiding this comment.
I wanted to point out why ordering is needed here not that Config.licenses is unordered. I agree that the original one is too wordy. I have shorten it to just one line. Additionally, I have removed the change to the following docstring.
Please take another look.
8333f7b to
1a304c4
Compare
`LicenseType` was built by iterating `Config.licenses`, which is a `set`. The order of iterating a `set` of `Enum` members depends on `PYTHONHASHSEED`, which CPython randomizes per process, so the member order of `LicenseType` varied from process to process. That member order is what fixes the order of the elements in the `enum` array of the `LicenseType` definition under `$defs`, which is emitted into all four generated JSON Schemas (`dandiset.json`, `published-dandiset.json`, `asset.json`, `published-asset.json`). Two invocations of `tools/pubschemata.py` on the same commit could therefore emit different files. Sorting the members by value at the point where the order becomes observable makes the generated schemas reproducible. `Config.licenses` remains a `set`, which is the right type for a configuration value that should be deduplicated and compared order-insensitively. Validation behavior is unchanged, since JSON Schema `enum` is order-insensitive and nothing in the code depends on the member order. Closes #433 Co-Authored-By: Claude Code 2.1.220 / Claude Opus 5 claude-opus-5 <noreply@anthropic.com>
1a304c4 to
0114c13
Compare
Fixes #433.
Problem
LicenseTypeis built by iteratingConfig.licenses, which is aset. The order of iterating asetofEnummembers depends onPYTHONHASHSEED, which CPython randomizes per process, so the member order ofLicenseTypevaried from process to process.That member order is what fixes the order of the elements in the
enumarray of theLicenseTypedefinition under$defs, which is emitted into all four generated JSON Schemas (dandiset.json,published-dandiset.json,asset.json,published-asset.json). Two invocations oftools/pubschemata.pyon the same commit could therefore emit different files.Fix
Sort the members by value when building the enum.
Config.licensesstays aset, with the sort applied at the single point where the order becomes observable. Validation behavior is unchanged, since JSON Schemaenumis order-insensitive.Merge order
Recommend merging to master before #419 is released. This PR carries no
DANDI_SCHEMA_VERSIONbump, since #419 already bumps it to0.8.0. The fix settles the order on["spdx:CC-BY-4.0", "spdx:CC0-1.0"], which differs from whatdandi/schemareleases/0.7.0/holds, so merging ahead of #419's release lets0.8.0be published once with the sorted order.Test plan
LicenseTypemember order is identical acrossPYTHONHASHSEEDin{0, 1, 2, 3, 4, 5, 42, 1234}tools/pubschemata.pyoutput is byte-identical between seeds 0 and 3; before the fix, all four schema files differedpytest dandischema: 258 passed, 19 skippedpre-commitandmypyclean