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

[MISC] move schema documentation into the schema directory #740

Merged
merged 3 commits into from
Apr 6, 2021

Conversation

Remi-Gau
Copy link
Collaborator

fixes #716

  • add a README in the schema folder
  • move some of the schema related content to that README and only keep in contributing.md info about how to modify the schema

@Remi-Gau Remi-Gau requested a review from tsalo February 21, 2021 14:17
@Remi-Gau
Copy link
Collaborator Author

FYI: the schema macro complains about the README when building the doc. I suspect this is expected given the python code expects only .yml but this may confuse people in the future.

@tsalo
Copy link
Member

tsalo commented Feb 21, 2021

FYI: the schema macro complains about the README when building the doc. I suspect this is expected given the python code expects only .yml but this may confuse people in the future.

I've gotten similar complaints about .DS_Store files on my Mac. Perhaps we should have load_schema() just ignore other file types, instead of raising warnings about them.

src/schema/README.md Outdated Show resolved Hide resolved
src/schema/README.md Outdated Show resolved Hide resolved
@Remi-Gau
Copy link
Collaborator Author

I've gotten similar complaints about .DS_Store files on my Mac. Perhaps we should have load_schema() just ignore other file types, instead of raising warnings about them.

Good idea.

Comment on lines +6 to +8
Currently, the only portion of the specification that relies on this schema is
the Entity Table, but any changes to the specification should be mirrored in the
schema.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not just the entity table, but also the entity definitions appendix and filename patterns throughout the specification.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to see if I can do a pull request about this suggestion I made for renaming the entity field (to key) in the entity.yaml file. But I have a hard time to find all the places that would be affected by this change, can you perhaps tell me, how do I test this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect most of the changes to be done there will be in the tools/schema_code, right @tsalo ?

Should we add a README into the tools folder with at least some very brief explanation of what to find in there?

Copy link
Member

@tsalo tsalo Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the code in that tools folder is where you will get the best idea of what parts of the schema are actively in use. It would be useful to add a README to that folder for right now, but I think we'll be moving that code into its own repository in the long run.

An alternative would be to focus on documenting the functions that are directly used in the specification (i.e., macros.py).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I can try checking the code base and improving the doc strings.

src/schema/README.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@Remi-Gau Remi-Gau changed the title [DOC] move schema documnetation into the schema folder [DOC] move schema documentation into the schema folder Feb 23, 2021
@tsalo tsalo added the schema Issues related to the YAML schema representation of the specification. Patch version release. label Mar 17, 2021
@sappelhoff
Copy link
Member

fyi @Remi-Gau, I applied Taylor's comments and rebased this on master so if you want to continue on this locally, you may want to run git fetch and git reset --hard origin/remi-schema-doc (provided that you don't have non-committed local changes)

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm quite happy with this, and would like to build off of it in #762. Hopefully we can merge soon.

@tsalo tsalo mentioned this pull request Apr 5, 2021
12 tasks
Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Taylor is happy with this, so am I, from my side, we can merge.

@tsalo tsalo merged commit 3301ac2 into bids-standard:master Apr 6, 2021
@Remi-Gau Remi-Gau deleted the remi-schema-doc branch November 15, 2021 05:30
@sappelhoff sappelhoff changed the title [DOC] move schema documentation into the schema folder [DOC] move schema documentation into the schema directory Mar 29, 2022
@sappelhoff sappelhoff changed the title [DOC] move schema documentation into the schema directory [MISC] move schema documentation into the schema directory Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema Issues related to the YAML schema representation of the specification. Patch version release.
Projects
Development

Successfully merging this pull request may close these issues.

Introductory info about the BIDS schema
4 participants