Skip to content

fix: bind ORM path values by field name - #500

Merged
jacebrowning merged 1 commit into
jacebrowning:mainfrom
arindamsikder:fix/orm-path-field-order
Oct 17, 2026
Merged

jacebrowning merged 1 commit into
jacebrowning:mainfrom
arindamsikder:fix/orm-path-field-order

Conversation

@arindamsikder

Copy link
Copy Markdown
Contributor

Summary

  • Restore models from path placeholders independently of dataclass field declaration order.
  • Cover all() and filter(), intervening non-path fields, recursive paths, exclusions, and alternate patterns with defaults.

Problem

Manager.all() discards the names returned by parse() and passes path values positionally to get(). When declaration order differs from placeholder order, values bind to the wrong fields and loading raises FileNotFoundError. Non-path fields before path fields have the same problem.

Solution

Retain the existing path-value normalization and exclusion checks, then associate each value with its parsed field name and call get() with keyword arguments. This follows the approach invited in #334. Public positional get() behavior is unchanged; filter() shares the corrected all() path. No type-conversion change for #147 is included.

Testing

Executed in a credential-free, network-isolated Linux sandbox with CPython 3.11.15 and lock-pinned test/quality dependencies:

  • Initial regression: 2 failures on original code, then 2 passes after the fix.
  • python -m pytest tests/test_orm_usage.py datafiles/tests/test_manager.py -q -p no:cacheprovider --basetemp=/tmp/pytest: 27 passed, 1 known xfail. Restoring the original manager.py with the final tests gives 5 failed, 22 passed, 1 xfail; restoring the fix passes again.
  • CI's repeated randomized suite: python -m pytest --count=2 --random --exitfirst --cov-report=xml -q -p no:cacheprovider --basetemp=/tmp/pytest: baseline 604 passed; fixed 616 passed. Both have 2 macOS-only skips and 10 existing xfails. Each run reports one cache_dir configuration warning because the cache plugin is disabled for isolation.
  • Baseline and fixed python -m black --check datafiles tests, python -m isort --check-only datafiles tests, python -m mypy datafiles tests --config-file=.mypy.ini (45 files), python -m pylint datafiles tests --rcfile=.pylint.ini, and python -m pydocstyle datafiles tests: pass.
  • Baseline and fixed python -m mkdocs build --clean --strict: pass.
  • All 9 notebooks executed with papermill.execute_notebook in both baseline and fixed disposable checkouts: pass. Kernel TCP warnings are present inside the private network namespace.
  • Baseline and fixed wheel/sdist builds via the configured poetry.core.masonry.api backend: pass; packaged runtime bytes verified.
  • git diff --check, added-line security scan, and independent automated diff review: pass.

Other Python versions and native Windows/macOS were not run locally; hosted CI still needs to validate its matrix. No CI approval controls were bypassed.

Related Issue

Fixes #334

Automation disclosure

Implemented, tested, and submitted by Hermes DEV (hermes:dev) using OpenAI gpt-6-astra, with a separate automated reviewer. No human pre-review or manual validation is claimed.

[verified] Independent automated diff review completed.
@jacebrowning
jacebrowning self-requested a review October 17, 2026 15:33
@jacebrowning
jacebrowning merged commit 8129442 into jacebrowning:main Oct 17, 2026
7 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.

Fields declaration order really matters when using multi-variables pattern

2 participants