fix: DeprecationWarning when using snakemake.utils.validate#3420
Conversation
📝 WalkthroughWalkthroughThe changes modify the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Validate as "validate()"
participant Resource as "Resource.from_contents"
participant Registry as "Registry"
participant Validator as "Draft202012Validator"
Caller->>Validate: Call validate(data, schema)
Validate->>Resource: Create resource from schema contents
Resource->>Registry: Associate resource with schema URI
Validate->>Validator: Instantiate validator with schema and registry
Validator->>Validate: Perform validation (_validate_record)
Validate->>Caller: Return validation result or warning
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.py`: Do not try to improve formatting. Do not suggest ...
⏰ Context from checks skipped due to timeout of 90000ms (31)
🔇 Additional comments (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/snakemake/utils.py (2)
94-103: Avoid overshadowing your initial Defaultvalidator assignment.You assign
Defaultvalidator = extend_with_default(Draft202012Validator)at line 94 and unconditionally reassign it withextend_with_default(Validator)at line 103. The first assignment is thereby overwritten every time. Consider moving or removing the line 94 assignment unless you truly need both.
107-110: Avoid repeated instantiation of validators for each row.Inside
_validate_record(), creatingDefaultvalidator(...)orValidatoron each call may impact performance when iterating over large data frames. Consider instantiating and caching validator objects outside the row-by-row loop (e.g., in_validate_pandasor_validate_polars) to optimize.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (1)
src/snakemake/utils.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
src/snakemake/utils.py
⏰ Context from checks skipped due to timeout of 90000ms (33)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
- GitHub Check: apidocs
🔇 Additional comments (4)
src/snakemake/utils.py (4)
48-51: Good introduction of direct imports for referencing.These additional imports from
snakemake.sourcecache,jsonschema, andreferencingalign well with your goal of replacing the oldRefResolverapproach. No particular issues stand out here.
66-70: New referencing-based validation looks solid.Instantiating a
Resourcefrom the schema and associating it with aRegistryforDraft202012Validatoris a recommended pattern for modern JSON Schema validation. This correctly removes the legacy RefResolver usage.
81-86: Loop logic for yielding validation errors appears correct.Extending the default properties validator and delegating errors through
validate_propertiesis a standard approach injsonschema.
89-92: Validator extension strategy is appropriate.Calling
validators.extend(...)here cleanly combines property default handling into the validator.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/snakemake/utils.py (2)
97-97: Be consistent with naming conventions.The variable name change from
DefaultValidatortoDefaultvalidatoris inconsistent with Python's convention of using CamelCase for classes. However, this isn't a class but a function, so it should follow snake_case.- Defaultvalidator = extend_with_default(Draft202012Validator) + default_validator = extend_with_default(Draft202012Validator)
106-106: Possible redundant assignment.The
Defaultvalidatorvariable is reassigned here after being assigned in line 97. This pattern is only necessary if the conditional at line 98 should override the validator when schema versions don't match. Consider refactoring to make the intent clearer.- Defaultvalidator = extend_with_default(Draft202012Validator) - if Validator.META_SCHEMA["$schema"] != schema["$schema"]: - logger.warning( - f"No validator found for JSON Schema version identifier '{schema['$schema']}'" - ) - logger.warning( - f"Defaulting to validator for JSON Schema version '{Validator.META_SCHEMA['$schema']}'" - ) - logger.warning("Note that schema file may not be validated correctly.") - Defaultvalidator = extend_with_default(Validator) + if Validator.META_SCHEMA["$schema"] != schema["$schema"]: + logger.warning( + f"No validator found for JSON Schema version identifier '{schema['$schema']}'" + ) + logger.warning( + f"Defaulting to validator for JSON Schema version '{Validator.META_SCHEMA['$schema']}'" + ) + logger.warning("Note that schema file may not be validated correctly.") + + Defaultvalidator = extend_with_default(Validator)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/snakemake/utils.py(3 hunks)tests/test_schema.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
src/snakemake/utils.pytests/test_schema.py
🔇 Additional comments (8)
tests/test_schema.py (2)
21-22: Excellent schema versioning update!Adding the
$schemadeclaration toBAR_SCHEMAensures compatibility with the updated validation mechanism that now uses the Draft 2020-12 validator.
30-30: Good JSON schema versioning!Adding the
$schemakey toBAR_JSON_SCHEMAcorrectly aligns with the updated validation approach in utils.py.src/snakemake/utils.py (6)
47-50: Good replacement of deprecated RefResolver with referencing library.The imports correctly include the necessary components from the
jsonschemaandreferencinglibraries needed to address the deprecation warning mentioned in issue #2468.
66-74: Great implementation of referencing library!This is a solid implementation using the modern approach with
ResourceandRegistryclasses from the referencing library. Theretrieve_urifunction provides a clean way to handle schema references.
84-95: Clean code formatting for validation properties.The formatting changes improve readability while maintaining the same functionality.
100-103: Fixed f-string syntax for dictionary access.The f-string syntax for accessing dictionary keys with single quotes has been correctly fixed, addressing the issue mentioned in the previous review.
110-113: Successfully updated validation calls.The validation calls now correctly use the new registry-based approach. This should effectively address the deprecation warning when using the
validatefunction.
47-113:Details
❓ Verification inconclusive
Verify the fix addresses the deprecation warning.
While the implementation looks correct, it would be good to confirm that the specific deprecation warning mentioned in issue #2468 is resolved by this change.
🏁 Script executed:
#!/bin/bash # Check if there are any tests that might trigger the validation functionality # Find all tests in the test directory that might use validate function echo "Looking for tests that use the validate function:" rg -A 2 -B 2 "validate\(" tests/ # Run a basic test case that involves validation to verify the fix echo "Running a simple test case to verify the fix:" python -c " from snakemake.utils import validate import json # Create a simple schema with $schema declaration schema = '{\"$schema\": \"https://json-schema.org/draft/2020-12/schema#\", \"type\": \"object\", \"properties\": {\"foo\": {\"type\": \"string\"}}}' with open('test_schema.json', 'w') as f: f.write(schema) # Create data to validate data = {'foo': 'bar'} # Run validation (should not produce deprecation warnings) validate(data, 'test_schema.json') print('Validation successful, check if any deprecation warnings were printed above.') " # Clean up rm -f test_schema.jsonLength of output: 960
Action Required: Ensure the test environment resolves module import issues before verifying deprecation fix
The current test run intended to check that the deprecation warning (issue #2468) is no longer triggered could not complete because it failed to locate the
snakemakemodule. Please ensure that the testing environment is set up correctly (for example, by configuring the PYTHONPATH or installing the package in a virtual environment) so that the module can be imported. Once the environment issue is resolved, re-run the test case to confirm that the deprecation warning no longer appears.
- Verify the repository's module path or install the
snakemakemodule.- Re-run the validation test to check for absence of deprecation warnings.
|
Thanks a lot, and sorry for the delay! |
🤖 I have created a release *beep* *boop* --- ## [9.6.0](v9.5.1...v9.6.0) (2025-06-16) ### Features * Prefer papermill to nbconvert ([#2857](#2857)) ([4263b03](4263b03)) ### Bug Fixes * DeprecationWarning when using snakemake.utils.validate ([#3420](#3420)) ([cf72427](cf72427)) * display group jobs on dryrun ([#3435](#3435)) ([3bebef4](3bebef4)) * expandvars for special profile keys ([#3597](#3597)) ([4020188](4020188)) * fix bug causing --precommand to not being executed before each remote job ([#3625](#3625)) ([e59d125](e59d125)) * improved toggle switch behavior in reports ([#3623](#3623)) ([0c4bd23](0c4bd23)) * pass envvars defined via the envvars directive to remote jobs ([#3626](#3626)) ([d4890b4](d4890b4)) * remove wms arg, update logging cli docs ([#3622](#3622)) ([3a9a5ac](3a9a5ac)) * typo in CondaEnvDirSpec.__eq__ (issue [#3192](#3192)) ([#3613](#3613)) ([f4c107f](f4c107f)) * Unclear handling of params overriding with inheritance ([#3624](#3624)) ([077ac4a](077ac4a)) ### Documentation * Added snakemake command to execute the rule plot_with_python ([#3608](#3608)) ([bd99c11](bd99c11)) * Updated Reporting Section of The Tutorial(Interaction, Visualization, and Reporting with Snakemake) ([#3606](#3606)) ([91e90ba](91e90ba)) * Updated Requirement Section of The Tutorial With Warning of Not Installing The Tools Manually ([#3607](#3607)) ([3bd114b](3bd114b)) * Updated Wrapper Version in Tutorial and Used Simple Rule For Consistency & Ease ([#3605](#3605)) ([b3bcc21](b3bcc21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
|
This broke a workflow of mine. I would appreciate some guidance on what I am doing wrong: Toy examplels -Rcat workflow/schemas/config.schema.yaml$schema: https://json-schema.org/draft/2020-12/schema
type: object
properties:
foo: { type: string }
bar: { $ref: bar.schema.yaml }cat workflow/schemas/bar.schema.yaml$schema: https://json-schema.org/draft/2020-12/schema
type: integercat config/config.yamlfoo: foo
bar: 42cat workflow/Snakefilefrom snakemake.utils import validate
configfile: "config/config.yaml"
validate(config, schema="schemas/config.schema.yaml")
rule all:
run: print(f"{config['foo']}={config['bar']}")check-jsonschema --check-metaschema workflow/schemas/*.schema.yamlPrevious behavioursnakemake --versionsnakemake -j 1 -q allNew behavioursnakemake --versionsnakemake -j 1 -q all |
|
@johanneskoester: Let me know if you prefer a separate issue for the regression I reported above. |
|
@WardDeb: Do you agree this regression was introduced by your change? Do you see anything wrong with my code? This completely broke several of my pipelines so I am stuck at Snakemake < 9.6.0 if I cannot find a way around this. |
|
I opened #3648 to track the issue I reported above independently of this closed pull request. |
| urljoin("file:", schemafile.get_path_or_uri()), | ||
| schema, | ||
| handlers={ | ||
| "file": lambda uri: _load_configfile(re.sub("^file://", "", uri)) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This commit fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in snakemake#3420. The two required changes were: 1. Injection of an absolute file:// $id in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a file:// URI). In addition, to prevent future regressions, this commit adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This commit intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this commit always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) where successfully run at commit `960f6a89eaa31da6014e810dfcf08f635ac03a6e` (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit `cf724272eefcd9b30fb625d2e16380727bef9c3e` (direct child of the above, introducing the changes from PR snakemake#3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit `f1517e83a6a4a193a1148543fc06177eeb11ecb5` (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this commit, validating it fixes the regression. --- fixes snakemake#3648
This commit fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in snakemake#3420. The two required changes were: 1. Injection of an absolute file:// $id in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a file:// URI). In addition, to prevent future regressions, this commit adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This commit intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this commit always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) where successfully run at commit `960f6a89eaa31da6014e810dfcf08f635ac03a6e` (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit `cf724272eefcd9b30fb625d2e16380727bef9c3e` (direct child of the above, introducing the changes from PR snakemake#3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit `f1517e83a6a4a193a1148543fc06177eeb11ecb5` (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this commit, validating it fixes the regression. --- fixes snakemake#3648
This commit fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in snakemake#3420. The two required changes were: 1. Injection of an absolute file:// $id in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a file:// URI). In addition, to prevent future regressions, this commit adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This commit intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this commit always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) where successfully run at commit `960f6a89eaa31da6014e810dfcf08f635ac03a6e` (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit `cf724272eefcd9b30fb625d2e16380727bef9c3e` (direct child of the above, introducing the changes from PR snakemake#3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit `a9c9d275f1f6a2069b78f79db01f718e2ea16734` (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this commit, validating it fixes the regression. --- fixes snakemake#3648
This commit fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in snakemake#3420. The two required changes were: 1. Injection of an absolute file:// $id in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a file:// URI). In addition, to prevent future regressions, this commit adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This commit intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this commit always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) were successfully run at commit `960f6a89eaa31da6014e810dfcf08f635ac03a6e` (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit `cf724272eefcd9b30fb625d2e16380727bef9c3e` (direct child of the above, introducing the changes from PR snakemake#3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit `a9c9d275f1f6a2069b78f79db01f718e2ea16734` (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this commit, validating it fixes the regression. --- fixes snakemake#3648
This commit fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in snakemake#3420. The two required changes were: 1. Injection of an absolute file:// $id in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a file:// URI). In addition, to prevent future regressions, this commit adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This commit intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this commit always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) were successfully run at commit `960f6a89eaa31da6014e810dfcf08f635ac03a6e` (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit `cf724272eefcd9b30fb625d2e16380727bef9c3e` (direct child of the above, introducing the changes from PR snakemake#3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit `a9c9d275f1f6a2069b78f79db01f718e2ea16734` (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this commit, validating it fixes the regression. --- fixes snakemake#3648
This commit fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in snakemake#3420. The two required changes were: 1. Injection of an absolute file:// $id in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a file:// URI). In addition, to prevent future regressions, this commit adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This commit intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this commit always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) were successfully run at commit `960f6a89eaa31da6014e810dfcf08f635ac03a6e` (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit `cf724272eefcd9b30fb625d2e16380727bef9c3e` (direct child of the above, introducing the changes from PR snakemake#3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit `2eb9079dae29f271aeacff75e59f6c2f6a0d352d` (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this commit, validating it fixes the regression. --- fixes snakemake#3648
This PR fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in #3420. The two required changes were: 1. Injection of an absolute `file://` `$id` in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a `file://` URI). In addition, to prevent future regressions, this PR adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This PR intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this PR always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) were successfully run at commit [`960f6a89eaa31da6014e810dfcf08f635ac03a6e`](960f6a8) (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit [`cf724272eefcd9b30fb625d2e16380727bef9c3e`](cf72427) (direct child of the above, introducing the changes from PR #3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit [`2eb9079dae29f271aeacff75e59f6c2f6a0d352d`](2eb9079) (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this PR, validating it fixes the regression. ### QC * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). #### Notes: This is my first contribution to the project. Thus, I am not familiar with the codebase. It is also my first time dealing with `jsonschema` and the both, its (deprecated) `RefResolver` API (to analyse the old implementation and design tests accordingly), and its current `resolving` API (to fix the regression). Please let me know if you have any adjustments you want/need me to do. Also, I started from a standalone POC script to make sure I understand the API, defined tests and then worked on the actual fix off that. Initially, I overestimated the power of pre-9.6.0's implementation of the `validate` function and designed a test that tested defaults propagation across (nested) `$ref`s. Thus, my intermediate solution was much more complex (and powerful) than this simple (minimal) fix. I do have a local branch with that work in case anybody is interested in implementing more powerful defaults value handling during config validation. Please reach out if there is interest. --- fixes #3648 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Schema resolution now enforces local resolution of relative $ref references, improving validation and default assignment across nested refs, fragments, allOf/anyOf/defs; may change behavior for schemas that previously relied on remote resolution. * **Tests** * Added comprehensive tests and fixtures covering relative/fragment references, nested schemas, allOf/anyOf/defs, default handling, and error scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…e#3420) This PR closes snakemake#2468, by replacing RefResolver functionality with the referencing library. ### QC <!-- Make sure that you can tick the boxes below. --> * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Modernized the data validation process by adopting the latest features of the JSON Schema Draft 2020-12 specification for improved reliability and consistency. - Added schema versioning information to existing schema definitions for enhanced validation and interoperability. - **Style** - Enhanced the formatting of warning messages for clearer and more readable notifications. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [9.6.0](snakemake/snakemake@v9.5.1...v9.6.0) (2025-06-16) ### Features * Prefer papermill to nbconvert ([snakemake#2857](snakemake#2857)) ([4263b03](snakemake@4263b03)) ### Bug Fixes * DeprecationWarning when using snakemake.utils.validate ([snakemake#3420](snakemake#3420)) ([cf72427](snakemake@cf72427)) * display group jobs on dryrun ([snakemake#3435](snakemake#3435)) ([3bebef4](snakemake@3bebef4)) * expandvars for special profile keys ([snakemake#3597](snakemake#3597)) ([4020188](snakemake@4020188)) * fix bug causing --precommand to not being executed before each remote job ([snakemake#3625](snakemake#3625)) ([e59d125](snakemake@e59d125)) * improved toggle switch behavior in reports ([snakemake#3623](snakemake#3623)) ([0c4bd23](snakemake@0c4bd23)) * pass envvars defined via the envvars directive to remote jobs ([snakemake#3626](snakemake#3626)) ([d4890b4](snakemake@d4890b4)) * remove wms arg, update logging cli docs ([snakemake#3622](snakemake#3622)) ([3a9a5ac](snakemake@3a9a5ac)) * typo in CondaEnvDirSpec.__eq__ (issue [snakemake#3192](snakemake#3192)) ([snakemake#3613](snakemake#3613)) ([f4c107f](snakemake@f4c107f)) * Unclear handling of params overriding with inheritance ([snakemake#3624](snakemake#3624)) ([077ac4a](snakemake@077ac4a)) ### Documentation * Added snakemake command to execute the rule plot_with_python ([snakemake#3608](snakemake#3608)) ([bd99c11](snakemake@bd99c11)) * Updated Reporting Section of The Tutorial(Interaction, Visualization, and Reporting with Snakemake) ([snakemake#3606](snakemake#3606)) ([91e90ba](snakemake@91e90ba)) * Updated Requirement Section of The Tutorial With Warning of Not Installing The Tools Manually ([snakemake#3607](snakemake#3607)) ([3bd114b](snakemake@3bd114b)) * Updated Wrapper Version in Tutorial and Used Simple Rule For Consistency & Ease ([snakemake#3605](snakemake#3605)) ([b3bcc21](snakemake@b3bcc21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR fixes a regression introduced in Snakemake 9.6.0 where relative `$ref` references in local config schemas were not correctly resolved during validation. This regression was introduced when refactoring the config validation from the (deprecated) `RefResolver` to the (current) `resolving` API in snakemake#3420. The two required changes were: 1. Injection of an absolute `file://` `$id` in schemas to enforce proper local reference resolution. 2. A follow-up update to retrieve_uri to correctly load subschemas (as it now receives a `file://` URI). In addition, to prevent future regressions, this PR adds tests for (nested) `$ref`s, `$ref` with remote `$id` (see below), fragment references, `allOf`/`anyOf` defaults, and `$def`s. This commit restores pre-9.6.0 behavior and prevents ValidationError when nested schemas are referenced. Caveat: This PR intentionally does *not* restore one specific aspect of the old (`RefResolver` based) implementation: If a schema file contains an explicit `$id` pointing to a remote copy of the schema, the old implementation would fetch any references (in- or external) from that remote URI. Instead, the implementation introduced by this PR always resolves references based on the local schema file. This behaviour is demonstrated (and validated) with the remote `$id` test mentioned above. The following checks have been performed to ensure the correctness of this patch: 1. All tests (but the remote `$id` one, see above) were successfully run at commit [`960f6a89eaa31da6014e810dfcf08f635ac03a6e`](snakemake@960f6a8) (last ancestor of current `main` that still uses the old `RefResolver` implementation), proving that they cover pre-9.6.0 behaviour. 2. Re-running the same tests at commit [`cf724272eefcd9b30fb625d2e16380727bef9c3e`](snakemake@cf72427) (direct child of the above, introducing the changes from PR snakemake#3420), the three (nested) `$ref`s test fail, verifying this commit/PR caused the regression and the tests added in this commit catch that regression. 3. The same tests still fail at commit [`2eb9079dae29f271aeacff75e59f6c2f6a0d352d`](snakemake@2eb9079) (current `main`), demonstrating that the regression has not been fixed yet. 4. All tests pass after applying the changes from this PR, validating it fixes the regression. ### QC * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). #### Notes: This is my first contribution to the project. Thus, I am not familiar with the codebase. It is also my first time dealing with `jsonschema` and the both, its (deprecated) `RefResolver` API (to analyse the old implementation and design tests accordingly), and its current `resolving` API (to fix the regression). Please let me know if you have any adjustments you want/need me to do. Also, I started from a standalone POC script to make sure I understand the API, defined tests and then worked on the actual fix off that. Initially, I overestimated the power of pre-9.6.0's implementation of the `validate` function and designed a test that tested defaults propagation across (nested) `$ref`s. Thus, my intermediate solution was much more complex (and powerful) than this simple (minimal) fix. I do have a local branch with that work in case anybody is interested in implementing more powerful defaults value handling during config validation. Please reach out if there is interest. --- fixes snakemake#3648 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Schema resolution now enforces local resolution of relative $ref references, improving validation and default assignment across nested refs, fragments, allOf/anyOf/defs; may change behavior for schemas that previously relied on remote resolution. * **Tests** * Added comprehensive tests and fixtures covering relative/fragment references, nested schemas, allOf/anyOf/defs, default handling, and error scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
The `referencing` dependency was introduced in this pull request: snakemake/snakemake#3420 It was released in version `9.6.0`: snakemake/snakemake#3611
A `referencing` dependency was added in snakemake version 9.6.0: snakemake/snakemake#3420 This was not added to the bioconda recipe of `snakemake-minimal` until version 9.19.0: #64534 So this PR patches the repodata accordingly.
* fix: snakemake depends on referencing since 9.6.0 The `referencing` dependency was introduced in this pull request: snakemake/snakemake#3420 It was released in version `9.6.0`: snakemake/snakemake#3611 * fix: bump build number for snakemake recipe update
….0 (#64539) * fix: add `referencing` dependency for snakemake-minimal >=9.6.0,<9.19.0 A `referencing` dependency was added in snakemake version 9.6.0: snakemake/snakemake#3420 This was not added to the bioconda recipe of `snakemake-minimal` until version 9.19.0: #64534 So this PR patches the repodata accordingly. * fix: also patch build 0 of version 9.19.0 * fix: do proper semantic version comparisons for newly added patch * fix: consistently use `parse_version()` for any semantic version comparison of version strings I manually tested that this does not change the patches that are created. I think we just got lucky in the past, in that none of the version numbers went above 9, so that lexicographical sort/comparison just always worked out. But we should enforce the usage, so that copy-pasted comparisons ensure this works for any version number ranges.
* fix: snakemake depends on referencing since 9.6.0 The `referencing` dependency was introduced in this pull request: snakemake/snakemake#3420 It was released in version `9.6.0`: snakemake/snakemake#3611 * fix: bump build number for snakemake recipe update
….0 (bioconda#64539) * fix: add `referencing` dependency for snakemake-minimal >=9.6.0,<9.19.0 A `referencing` dependency was added in snakemake version 9.6.0: snakemake/snakemake#3420 This was not added to the bioconda recipe of `snakemake-minimal` until version 9.19.0: bioconda#64534 So this PR patches the repodata accordingly. * fix: also patch build 0 of version 9.19.0 * fix: do proper semantic version comparisons for newly added patch * fix: consistently use `parse_version()` for any semantic version comparison of version strings I manually tested that this does not change the patches that are created. I think we just got lucky in the past, in that none of the version numbers went above 9, so that lexicographical sort/comparison just always worked out. But we should enforce the usage, so that copy-pasted comparisons ensure this works for any version number ranges.
This PR closes #2468, by replacing RefResolver functionality with the referencing library.
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).Summary by CodeRabbit