test: integration tests, edge cases, and docs - #113
Open
kaitj wants to merge 6 commits into
Open
Conversation
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Benchmark Results
|
kaitj
marked this pull request as ready for review
August 20, 2026 18:14
kaitj
force-pushed
the
feat/generalize-ents/phase5
branch
from
August 27, 2026 20:30
60680e8 to
0adb2c4
Compare
- Add a "Filtering files" section covering `--filter ENTITY=PATTERN` syntax, glob support, and OR-within-entity / AND-across-entities semantics, with working examples - Flag `--subjects` as deprecated with a pointer to `--filter sub=...` - Document `--schema` fla, which was previously covered only under the python api - Remove shell-prompt prefixes from examples, and tag fences as appropriate with `sh`
clear_schema_caches() did not actually force a re-read of a changed on-disk schema. bidsschematools.schema.load_schema is itself lru_cached on the schema path and sits upstream of every bids2table schema cache, so clearing only the downstream caches (_load_from_path, entity_arrow_schema, ...) just re-populated them from the still-warm load_schema cache — a stale adapter was returned. Clear load_schema's cache too (the root of the chain) so a changed schema file is genuinely re-resolved. Tests: - test_schema_switch_mid_session_uses_new_schema: regression guard proving a changed schema file is served stale until clear_schema_caches() is called, then re-read. - test_index_derivative_without_description: a derivative dataset under derivatives/ without dataset_description.json is still detected, typed as a derivative, and indexed.
Two filter-related coverage gaps from the "Generalize entities" effort:
- tests/test_main.py: test_main_index_filter_and_schema — runs `b2t2 index`
with BOTH `-f` and `--schema` in a single invocation (no test exercised the
pair before). Uses a modified copy of the default schema so the effect of
each flag is observable: asserts the filter reduces the row count (only
sub-A01) and that the custom schema's description reaches the output column
metadata via the parquet round-trip.
- tests/test_indexing.py: test_index_dataset_s3_filter — cloud-marked; indexes
s3://openneuro.org/ds000102 with filters={"sub": "01"} and asserts the filter
applies (5 rows, all sub-01), matching the local ds102 mirror. Skipped
automatically when cloudpathlib is unavailable.
Remove functions only reachable from tests: - _entities.py: get_entity_name, get_entity_regex, get_entity_glob_pattern - _indexing.py: _is_bids_entity_dir (one-line pattern.fullmatch wrapper) - their tests in test_entities.py and test_indexing.py Wire dataset_type into the index. It was set only from DatasetType in dataset_description.json, so derivatives without a declared type indexed as null. The description is now read once and enriched with a derived DatasetType (derivative under a nested parent dir, else raw); _get_dataset_type takes that description instead of re-reading it.
Consolidate two glob matchers that did the same job (match the bare value or the key-value form against a pattern): - _indexing.py: drop _match_entity_name, route its call site (_find_bids_entity_dirs) through _match_single; the include filter no longer round-trips through two name-sets. - Extract the shared `format -> char_class` lookup, used by both _build_datatype_pattern and _compile_entity_dir_pattern, into _schema._char_class_for.
Apply the public/private docstring convention across the package and drop inline comments that merely restate the code. No functional changes. - Public API: use structured Google docstrings; add `Returns:` where a function returns a value (e.g. `cloudpathlib_is_available`, `BIDSLayout.to_df`) - Private helpers: reduce to concise one-liners, stripping boilerplate `Args:`/`Returns:` scaffolding while keeping non-obvious notes in prose - Trim inline comments that restate the code; keep purpose/why/strategy comments (e.g. the legacy `database_path` note) - Fix stale `bids2table_compat` import examples to `bids2table.pybids` - Convert `listify` from numpydoc to Google style - Fix `compatability` -> `compatibility` typo
kaitj
force-pushed
the
feat/generalize-ents/phase5
branch
from
September 2, 2026 17:38
0adb2c4 to
12cac21
Compare
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
1 participant
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.
This PR:
Final phase of the generalized entities refactor, building on top of phase 4 (#112) to add testing, fix surfaced bugs, a cleanup pass, and documenting new flags.
--filter+--schemain one CLI invocation; S3 entity-filter; edge cases (derivative detection withoutdataset_description.json, and schema-switch re-indexingclear_schema_cachhes()now also clearsbidsschematools.schema.load_schema's own@lru_cache(previously downstream caches re-populated from still-warm upstream cache)dataset_typeinstead of leaving unset--filter+--schemain README; note deprecation of--subjects/include_subjectsType of Change
Related Issue(s)
n/a
Stack created with GitHub Stacks CLI • Give Feedback 💬