Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rf(context): Move file tree, schema, and issues into context.dataset #2066

Merged
merged 7 commits into from
Aug 7, 2024

Conversation

effigies
Copy link
Collaborator

@effigies effigies commented Aug 6, 2024

Dataset issues sensibly accumulate on the long-lived dataset object, instead of the ephemeral context object. dataset.tree is defined by the schema, and also makes sense there.

I think walk.ts demonstrates this well. The dataset context provides the files to be iterated over, and a new context can be generated with just the current file and the dataset context.

There's still room to pre-allocate a context.subject object and pass that through, but this is enough of a change for one PR, IMO.

This will help focus #2057 on the specific changes needed for those issues.

Copy link

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 79.41176% with 21 lines in your changes missing coverage. Please review.

Project coverage is 87.14%. Comparing base (12f40bb) to head (e710edd).
Report is 1 commits behind head on master.

Files Patch % Lines
bids-validator/src/schema/applyRules.ts 40.00% 9 Missing ⚠️
bids-validator/src/schema/context.ts 86.20% 8 Missing ⚠️
bids-validator/src/validators/filenameValidate.ts 72.72% 3 Missing ⚠️
bids-validator/src/validators/bids.ts 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2066      +/-   ##
==========================================
+ Coverage   85.69%   87.14%   +1.44%     
==========================================
  Files          91      139      +48     
  Lines        3782     6689    +2907     
  Branches     1220     1579     +359     
==========================================
+ Hits         3241     5829    +2588     
- Misses        455      770     +315     
- Partials       86       90       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@effigies effigies force-pushed the rf/datasetcontext branch from b72cc7f to 4020d66 Compare August 6, 2024 23:30
@effigies effigies force-pushed the rf/datasetcontext branch from f3e6012 to f2689ba Compare August 7, 2024 17:02
@effigies effigies changed the title rf(context): Move file tree and issues into context.dataset rf(context): Move file tree, schema, and issues into context.dataset Aug 7, 2024
@effigies effigies added the schema label Aug 7, 2024
Most notably, move context.fileTree to context.dataset.tree.

Reorganized the constructors, which had long tendrils, but the blast
radius was pretty small.
Dataset issues more sensibly accumulate on the long-lived dataset
object, instead of the throw-away context object.
@effigies effigies force-pushed the rf/datasetcontext branch from f2689ba to bc0a987 Compare August 7, 2024 17:25
@rwblair rwblair merged commit c98cb99 into bids-standard:master Aug 7, 2024
15 of 16 checks passed
@effigies effigies deleted the rf/datasetcontext branch August 15, 2024 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants