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

WIP add entities to context and validate them. #1456

Merged
merged 23 commits into from
May 27, 2022

Conversation

rwblair
Copy link
Member

@rwblair rwblair commented May 12, 2022

Issues with branch that I can think of right now:

  1. File organization, right now the code that checks the schema.rules.datatypes against the context entities is in schema/applyRules.ts, they should go in their own file. Should they live in the schema directory as opposed to validation? Should there be a directory or filename structure that matches what part of the spec is being checked.
  2. Issues - I have a place holder issue in applyRules.ts. I don't have a clear idea of how we want to mange issue creation and error codes. This part of the spec doesn't have error codes associated with it like schema.rules.checks does.
  3. Required top level files - will probably need to be its own check, since current checks are applied against each context separately.
  4. Need to generate hints at start of checkDatatype if we have a suffix match but no extension match.
  5. Need to validate datatype derived from match and compare it to the datatype directory in the actual path if one is present.
  6. use generated context types generated from specification repo when they're ready.
  7. Running with --no-check right now, outlook on having typing for everything?
  8. Need to write tests for functions added in applyRules and expand entities.test.ts tests.
  9. Need to catch if no rule matches, could be done by presence of datatype in context at end of checkDatatypes loop.
  10. deleting schema.rules.datatypes.derivatives currently. Will need to resolve $ref properly to understand it.

@rwblair
Copy link
Member Author

rwblair commented May 12, 2022

readEntities will need to be updated once decision from bids-standard/bids-specification#1088 is implemented.

@nellh
Copy link
Member

nellh commented May 17, 2022

1. File organization, right now the code that checks the schema.rules.datatypes against the context entities is in schema/applyRules.ts, they should go in their own file. Should they live in the schema directory as opposed to validation? Should there be a directory or filename structure that matches what part of the spec is being checked.

Since it's a lot more data driven, we should just aim for whatever is clearest for implementation over mirroring the specification structure.

2. Issues - I have a place holder issue in applyRules.ts. I don't have a clear idea of how we want to mange issue creation and error codes. This part of the spec doesn't have error codes associated with it like schema.rules.checks does.

We might need to just make some new error keys for this part. Since we already decided to move away from numerical keys, we might just want to decide on the format allowed for keys and enforce it as part of the Issue constructor. For OpenNeuro, I think we want to stick to the types we have for this already as much as we can but OpenNeuro doesn't rely on the numerical code except for configuration (easy to update).

3. Required top level files - will probably need to be its own check, since current checks are applied against each context separately.

I'm trying out some prototype chaining for the contexts so the leafs can just chain up to things like this. Maybe we should add a shorthand to the context object for the top level though since that seems pretty useful.

@rwblair
Copy link
Member Author

rwblair commented May 20, 2022

2. Issues - I have a place holder issue in applyRules.ts. I don't have a clear idea of how we want to mange issue creation and error codes. This part of the spec doesn't have error codes associated with it like schema.rules.checks does.

Latest commits change issue management to be an object and use module exports/imports then have an addIssue function to update it. This gets around having to return and destructure arrays of issues, and keeps issues organized by their code automatically. I used a simplified version of the existing issue types for ease of construction, but if they need to match the old type in any way can change them. Still just treating the code as any old string in issue creation.

@nellh
Copy link
Member

nellh commented May 24, 2022

I think we shouldn't change the Issue interface if possible since we have several tools that depend on it. Deprecating the numeric code should be fine but the other parts I'd like to keep.

@nellh nellh merged commit 2d5a069 into bids-standard:schema-prototyping May 27, 2022
@rwblair rwblair deleted the schema/entitiesParse branch August 7, 2024 18:15
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.

2 participants