-
Notifications
You must be signed in to change notification settings - Fork 67
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
DOC: Automatically document settings #863
Conversation
task_is_rest: bool = False | ||
""" | ||
Whether the task should be treated as resting-state data. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes like this are just copy-paste moves to match orders in .md
files above, no content changes.
mne_bids_pipeline/_config.py
Outdated
# ## Break detection | ||
# | ||
# --- | ||
# tags: | ||
# - preprocessing | ||
# - artifact-removal | ||
# - raw | ||
# - events | ||
# --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hoechenberger this is my one open question -- useful to have the tags
in the _config.py
or no? If not I'll move to the parsing .py
file instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you add them to the Python files? What would that look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the parsing gen file something like
tags = {
"break detection": ("preprocessing", "artifact-removal", "raw", "events"),
...
}
One tuple of tags per option category, then the parser adds them to the .md files in the right place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a clear preference here, I suppose I'm fine with either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it more I decided to move it to gen_settings.py
-- that way someday we could consider auto-generating / adding tags. And I think it keeps _config.py
a little bit more compact which is nice.
# This functionality will soon be removed from the pipeline, and | ||
# will be integrated into MNE-BIDS. | ||
# | ||
# "Bad", i.e. flat and overly noisy channels, can be automatically detected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocks of text like this were taken from the .md
files and moved to the _config.py
, which I think is a quality-of-life improvement for end users. We had little bits of this stuff in some places and some outdated info, and now it's documented nicely in the code and in the website the same way.
Even with all the parsing code we end up with a net-negative line count in the diff |
@larsoner I won't be able to thoroughly review for now; please go ahead and merge if you're happy and i'll start complaining sometime later 😁 |
Complaints always welcome, but even more welcome if they're in the form of a PR to improve wordings etc. 😄 |
In my view, it would be helpfull to explain the memory-related arguments, and more generally the caching behavior in a dedicated page, like the basic usage |
i agree!! |
This PR automatically adds configuration options from
_config.py
todocs/source/settings/
. It's a first step toward #862. To accomplish this, I wrote code to parse header levels#
,##
,###
and map those onto our current hierarchy. I made a few opinionated choices:Make a new
Execution
section that has stuff liken_jobs
. It seems useful to separate those from stuff likebids_root
which are more dataset-specific and actually affect what gets generated as opposed to how it gets generated on machines / jobs etc..Chose the website docs as the source of truth for parameter order. This means the
_config.py
diff is bigger, but the.md
diffs are smaller.Chose to add the
tags
to the_config.py
. This was a bit of a toss-up for me -- if it is not useful there I can easily just move those to a dict ingen_settings.py
.For now I have left the generated
.md
files in source control but this is just to facilitate review to see how the docs have changed. Once people are happy I willrm
and add to.gitignore
.Link to rendered doc: https://output.circle-artifacts.com/output/job/0a5e0fe7-0af8-4717-80f5-e33f99967966/artifacts/0/site/settings/general.html
Before merging …
docs/source/changes.md
).md
files that are autogenerated and add to.gitignore