Repository navigation
Refuse parameter rows of another scope than the settings' (#37) - #63
Merged
Merged
Conversation
Parameters.from_columns now refuses a normalization_scope that differs from settings.scope, as it already did for the method: parameters fitted for one sampling population are never read as another's. The check lives in parameter_checks.check_scope (keeps fitting.py under 600 lines). test_columns_round_trip_exactly now reads its patch rows with patch settings; new tests cover every row/settings scope pair and per-time-point groups. Fixes #37 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Owner decision on #37: strict.
Parameters.from_columnsnow refuses a row whosenormalization_scopediffers fromsettings.scope, just as it already refuses a differentnormalization_method.Fixes #37
Changes
parameter_checks.check_scope(scope, expected, scopes)refuses two kinds of scope:fitting._scopeis removed andfrom_columnscallscheck_scope. This keepsfitting.pyat 591 lines; adding the check inline would have taken it to 608.test_columns_round_trip_exactlyreads itspatchrows withNormalizationSettings(scope="patch").from_columnsrow and §9.1, AGENTS map.Evidence
ruff check .andruff format --check .pass.pytest: 610 passed, 2 skipped (csbdeep, which is not installed locally).Consumer impact (CellPatcher, when it repins past v0.3.2)
CellPatcher currently pins v0.3.2, so nothing changes for it until it repins.
cellpatcher/normalization.py::parameters_from_row(origin/main 79adc3a, lines 270-276) buildsNormalizationSettingswithoutscope. Against this branch, two CellPatcher tests fail withnormalization_scope is 'patch', but the settings use scope 'image':test_read_patch_table.py::test_an_exported_run_reads_back_as_its_patchestest_rgb_normalization.py::test_rgb_raw_export_and_lazy_delivery_give_the_eager_arraysCellPatcher must pass the run's scope:
I tried this in a scratch copy, not in the CellPatcher repository. Both failures are fixed, but
test_normalization.py::test_raw_patches_of_a_removed_scope_still_normalize_on_loadthen fails. That test'srun_config.jsonholds the removed scopecondition, soNormalizationSettings(scope="condition")raises.Real rows from a
condition- ortechnical_repeat-scope run hold that scope innormalization_scope. Since #62,from_columnsrefuses them as an unknown scope whatever settings are passed. For those runs, CellPatcher has two options:Parametersfrom the pair columns itself, as it already does for the legacy;-separated rows.Parameters(...)does not check the scope.Generated by Claude Code