Adapt index_reference and attr_reference to use glom#15
Adapt index_reference and attr_reference to use glom#15
index_reference and attr_reference to use glom#15Conversation
- Added `glom` to `dependencies` in `pyproject.toml` - Rewrote `index_reference` and `attr_reference` closures to use `glom` to fetch values. - `index_reference` explicitly walks paths using item accesses to emulate strict dictionary lookups - `attr_reference` defaults to `glom` standard `Path` lookups, which handles attributes correctly - Both gracefully map their `default` properties through to `glom` where applicable - Updated tests to catch `PathAccessError` or base `GlomError` depending on context where standard errors were formerly caught Co-authored-by: Ciemaar <1388496+Ciemaar@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
- Added `.pre-commit-config.yaml` to run `ruff` (with `--fix`) and `ruff-format`. - Updated `.github/workflows/tests.yml` to include `pre-commit` hook action. - Verified unused imports (`F401`) and import sorting (`I`) are successfully enabled in `ruff` inside `pyproject.toml` and verified them against modifications. Co-authored-by: Ciemaar <1388496+Ciemaar@users.noreply.github.com>
- Enforced `ruff` and `ruff-format` globally via `pre-commit run --all-files` - Addressed latent formatting issues in `Lightning Talk.ipynb`, `examples/mythica/model.py`, `examples/pycon.py`, `examples/simulate_characters.py`, `fix_type_hints.py`, `src/closure_collector/util.py`, and `test/test_hypothesis.py` (many of which were syntax depreciations like `%s` templating, unused lambda defs in `.ipynb` files, and `(A, B)` isinstance checks instead of `A | B`). - Ensures the new CI step (that mandates pre-commit tests) passes. Co-authored-by: Ciemaar <1388496+Ciemaar@users.noreply.github.com>
src/closure_collector/closures.py
Outdated
| return closure that references values stored elsewhere using glom. | ||
| :type flock: flock.core.FlockDict | ||
| :param indexes: lambdas to be resolved in order (tree walking) | ||
| :param indexes: keys or attributes to be resolved in order (tree walking) |
There was a problem hiding this comment.
This comment says keys or attributes, but the code seems to be limited to indexes, ensure that there are tests to prove the implemented behavior and update the docstring and comments.
There was a problem hiding this comment.
You're right, index_reference handles specifically item lookup. I've updated the docstring to correctly state it strictly resolves keys via item access (tree walking) and implemented extra tests enforcing index_reference fails if called against object attributes instead of dictionary keys.
There was a problem hiding this comment.
@jules Add index and attr reference testing by hypothesis, it should show that multiple levels of indexes or attrs can be traversed.
There was a problem hiding this comment.
Added! I generated a set of multiple-level Hypothesis tests that create variable depth object/dictionary trees and assert both attr_reference and index_reference accurately walk those generated paths using glom.
- Updates docstring for `index_reference` to accurately capture that it relies exclusively on key/item indexing instead of object attributes. - Appended unit tests specifically checking that `index_reference` throws `GlomError` (`PathAccessError`) against attribute retrieval. - Adds Hypothesis tests capable of creating variable depth dictionary/object graphs validating multi-level hierarchy evaluations. Co-authored-by: Ciemaar <1388496+Ciemaar@users.noreply.github.com>
- When evaluating the output of `index_reference` and `attr_reference`, Hypothesis may generate `nan` as a test value. Because `nan != nan`, we have to isolate tests against `float('nan')` via `math.isnan()` so comparisons properly resolve instead of failing CI checks.
Co-authored-by: Ciemaar <1388496+Ciemaar@users.noreply.github.com>
- Added `SESSION_INSTRUCTIONS.md` to record the sequence of requests, parameters, constraints, and fixes applied throughout this session for future context and reproducibility. Co-authored-by: Ciemaar <1388496+Ciemaar@users.noreply.github.com>
1 participant
This pull request adapts the
index_referenceandattr_referenceclosures withinclosure_collectorto rely on theglomlibrary.The
index_referencefunction iterates explicit item fetches by applying a custom path resolver viaglomto emulate dictionary key accesses.The
attr_referencefunction takes advantage of the nativePathspecifier built intoglomwhich correctly fetches object attributes as a default.Additionally, test cases within
test_closureshave been properly updated to handle exceptions thrown byglom, specificallyPathAccessErrorandGlomError.The
pyproject.tomlfile correctly incorporatesglomas a dependency.PR created automatically by Jules for task 8701495631153348954 started by @Ciemaar