juaml/junifer
Watch
3
0
Fork
You've already forked junifer
 0
Code Issues 14 Pull requests 2 Packages Activity Actions

[DOC]: Improve documentation #317

Merged
synchon merged 19 commits from update/improve-docs into main 2024-04-02 07:59:33 +00:00
Conversation 15 Commits 19 Files changed 27 +842 -338
synchon commented 2024-03-27 15:33:44 +00:00 (Migrated from github.com)
Copy link
  • description of feature/fix
  • tests added/passed
  • add an entry for the latest changes

This PR improves the documentation and in particular introduces space transformation via SpaceWarper and adds information about creating custom Preprocessors.

* [x] description of feature/fix * [x] tests added/passed * [x] add an entry for the latest changes This PR improves the documentation and in particular introduces space transformation via `SpaceWarper` and adds information about creating custom Preprocessors.
LeSasse (Migrated from github.com) reviewed 2024-03-27 15:33:44 +00:00
codecov[bot] commented 2024-03-27 15:37:09 +00:00 (Migrated from github.com)
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.15%. Comparing base (382eea7) to head (3c847a2).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #317   +/-   ##
=======================================
  Coverage   88.15%   88.15%           
=======================================
  Files         111      111           
  Lines        4820     4820           
  Branches      957      957           
=======================================
  Hits         4249     4249           
  Misses        417      417           
  Partials      154      154
Flag Coverage Δ
docs 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/317?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 88.15%. Comparing base [(`382eea7`)](https://app.codecov.io/gh/juaml/junifer/commit/382eea76071c50b5a3e76366f247116da0ee97ab?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) to head [(`3c847a2`)](https://app.codecov.io/gh/juaml/junifer/pull/317?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml). <details><summary>Additional details and impacted files</summary> [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/317/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://app.codecov.io/gh/juaml/junifer/pull/317?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #317 +/- ## ======================================= Coverage 88.15% 88.15% ======================================= Files 111 111 Lines 4820 4820 Branches 957 957 ======================================= Hits 4249 4249 Misses 417 417 Partials 154 154 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/317/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [docs](https://app.codecov.io/gh/juaml/junifer/pull/317/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `100.00% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more. </details>
github-actions[bot] commented 2024-03-27 15:41:03 +00:00 (Migrated from github.com)
Copy link
PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-04-02 08:04 UTC
[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.7 :---: Preview removed because the pull request was closed. 2024-04-02 08:04 UTC <!-- Sticky Pull Request Commentpr-preview -->
LeSasse (Migrated from github.com) requested changes 2024-03-28 13:10:45 +00:00
LeSasse (Migrated from github.com) left a comment
Copy link

Overall very strong improvements, I only have a few minor comments and questions.

Overall very strong improvements, I only have a few minor comments and questions.
docs/builtin.rst
@ -125,0 +131,4 @@
State: this should indicate the state of the preprocessor. Valid options are
- Planned
- In Progress
- Done
LeSasse (Migrated from github.com) commented 2024-03-28 12:56:11 +00:00
Copy link

Does Junifer handle/allow use of multiple preprocessors chained in sequence? how would parametrisation in the YAML look like for that?

Does Junifer handle/allow use of multiple preprocessors chained in sequence? how would parametrisation in the YAML look like for that?
docs/builtin.rst
@ -125,0 +169,4 @@
Planned
~~~~~~~
LeSasse (Migrated from github.com) commented 2024-03-28 12:57:00 +00:00
Copy link

Why particularly useful for fMRIPrep'ed data?

Why particularly useful for fMRIPrep'ed data?
docs/builtin.rst
@ -277,8 +321,8 @@ Available
* - Name
LeSasse (Migrated from github.com) commented 2024-03-28 12:57:41 +00:00
Copy link

Above you add Version Added with captial A. Similar for Template spaces, should it be Template Spaces?

Above you add Version Added with captial A. Similar for Template spaces, should it be Template Spaces?
docs/builtin.rst
@ -646,8 +690,8 @@ Available
LeSasse (Migrated from github.com) commented 2024-03-28 12:57:54 +00:00
Copy link

see above cmt

see above cmt
docs/builtin.rst
@ -663,14 +707,14 @@ Available
- | Vickery, Sam, & Patil, Kaustubh. (2022).
LeSasse (Migrated from github.com) commented 2024-03-28 12:58:32 +00:00
Copy link

you made a point to fix Junifer to junifer. Should it be nilearn as well rather than Nilearn?

you made a point to fix Junifer to junifer. Should it be nilearn as well rather than Nilearn?
docs/extending/dependencies.rst
@ -0,0 +1,141 @@
.. include:: ../links.inc
LeSasse (Migrated from github.com) commented 2024-03-28 13:00:12 +00:00
Copy link

Should dependencies be capitalised?

Should dependencies be capitalised?
LeSasse (Migrated from github.com) commented 2024-03-28 13:01:38 +00:00
Copy link

"having two keys" -> "with two keys"

"having two keys" -> "with two keys"
docs/extending/dependencies.rst
@ -0,0 +106,4 @@
mandatory to only allow the value of ``using`` argument to be one of them
specified in the ``using`` key of ``_CONDITIONAL_DEPENDENCIES`` entries.
For brevity, we only show the ``FSLWarper`` here but ``ANTsWarper`` looks very
LeSasse (Migrated from github.com) commented 2024-03-28 13:02:31 +00:00
Copy link

``ANTsWarper``` the capitalisation here makes me anxious, but I'll allow it

``ANTsWarper``` the capitalisation here makes me anxious, but I'll allow it
docs/extending/parcellations.rst
@ -52,14 +53,14 @@ first label in this list corresponds to the first integer label in the
parcellation and so on).
LeSasse (Migrated from github.com) commented 2024-03-28 13:04:36 +00:00
Copy link

"For example, a simple example could look like this:" -> "A simple example could look like this:"

"For example, a simple example could look like this:" -> "A simple example could look like this:"
docs/extending/preprocessor.rst
@ -0,0 +157,4 @@
...
Step 4: Finalise the Preprocessor
LeSasse (Migrated from github.com) commented 2024-03-28 13:07:17 +00:00
Copy link

I believe in multiple places I saw the american spelling of things, so this should be Finalize. Can we record in some central place i.e. something like "How to contribute to docs" that we aim to use the american spelling?

I believe in multiple places I saw the american spelling of things, so this should be Finalize. Can we record in some central place i.e. something like "How to contribute to docs" that we aim to use the american spelling?
synchon (Migrated from github.com) reviewed 2024-03-28 13:42:22 +00:00
docs/builtin.rst
@ -125,0 +131,4 @@
State: this should indicate the state of the preprocessor. Valid options are
- Planned
- In Progress
- Done
synchon (Migrated from github.com) commented 2024-03-28 13:42:22 +00:00
Copy link

Yeah it does. preprocess accepts a list now and the execution follows the sequence you specify in the YAML.

Yeah it does. `preprocess` accepts a list now and the execution follows the sequence you specify in the YAML.
synchon (Migrated from github.com) reviewed 2024-03-28 13:44:08 +00:00
docs/builtin.rst
@ -125,0 +169,4 @@
Planned
~~~~~~~
synchon (Migrated from github.com) commented 2024-03-28 13:44:08 +00:00
Copy link

From your issue description I understand that fMRIPrep doesn't perform smoothing after confound regression.

From your issue description I understand that fMRIPrep doesn't perform smoothing after confound regression.
synchon (Migrated from github.com) reviewed 2024-03-28 13:45:33 +00:00
docs/builtin.rst
@ -277,8 +321,8 @@ Available
* - Name
synchon (Migrated from github.com) commented 2024-03-28 13:45:33 +00:00
Copy link

Yeah missed it, good catch.

Yeah missed it, good catch.
synchon (Migrated from github.com) reviewed 2024-03-28 13:46:36 +00:00
docs/builtin.rst
@ -663,14 +707,14 @@ Available
- | Vickery, Sam, & Patil, Kaustubh. (2022).
synchon (Migrated from github.com) commented 2024-03-28 13:46:36 +00:00
Copy link

That would be better, fair point.

That would be better, fair point.
synchon (Migrated from github.com) reviewed 2024-03-28 13:50:03 +00:00
docs/extending/dependencies.rst
@ -0,0 +106,4 @@
mandatory to only allow the value of ``using`` argument to be one of them
specified in the ``using`` key of ``_CONDITIONAL_DEPENDENCIES`` entries.
For brevity, we only show the ``FSLWarper`` here but ``ANTsWarper`` looks very
synchon (Migrated from github.com) commented 2024-03-28 13:50:02 +00:00
Copy link

It's to follow the convention of the tool name like in other places in the code base.

It's to follow the convention of the tool name like in other places in the code base.
LeSasse (Migrated from github.com) reviewed 2024-03-28 13:51:16 +00:00
docs/extending/dependencies.rst
@ -0,0 +106,4 @@
mandatory to only allow the value of ``using`` argument to be one of them
specified in the ``using`` key of ``_CONDITIONAL_DEPENDENCIES`` entries.
For brevity, we only show the ``FSLWarper`` here but ``ANTsWarper`` looks very
LeSasse (Migrated from github.com) commented 2024-03-28 13:51:16 +00:00
Copy link

I understand :)

I understand :)
synchon (Migrated from github.com) reviewed 2024-03-28 13:53:18 +00:00
docs/extending/preprocessor.rst
@ -0,0 +157,4 @@
...
Step 4: Finalise the Preprocessor
synchon (Migrated from github.com) commented 2024-03-28 13:53:18 +00:00
Copy link

I've made everything follow British English in the docs. Recording it is a good idea.

I've made everything follow British English in the docs. Recording it is a good idea.
LeSasse (Migrated from github.com) reviewed 2024-03-28 13:54:34 +00:00
docs/extending/preprocessor.rst
@ -0,0 +157,4 @@
...
Step 4: Finalise the Preprocessor
LeSasse (Migrated from github.com) commented 2024-03-28 13:54:34 +00:00
Copy link

Ok, i will point out the american way when i find them.

Ok, i will point out the american way when i find them.
LeSasse (Migrated from github.com) reviewed 2024-03-28 13:56:14 +00:00
docs/extending/datagrabber.rst
@ -339,7 +356,7 @@ method, in the same order.
return ["subject", "session"]
LeSasse (Migrated from github.com) commented 2024-03-28 13:55:23 +00:00
Copy link

this is american spelling

this is american spelling
synchon (Migrated from github.com) reviewed 2024-03-28 15:08:44 +00:00
docs/extending/datagrabber.rst
@ -339,7 +356,7 @@ method, in the same order.
return ["subject", "session"]
synchon (Migrated from github.com) commented 2024-03-28 15:08:44 +00:00
Copy link

Boston tea party reversed.

Boston tea party reversed.
fraimondo (Migrated from github.com) approved these changes 2024-04-01 15:24:06 +00:00
fraimondo (Migrated from github.com) left a comment
Copy link

Just a typo, for the rest, I'm good.

Just a typo, for the rest, I'm good.
docs/using/codeless.rst
@ -5,7 +5,7 @@
Code-less Configuration
fraimondo (Migrated from github.com) commented 2024-04-01 15:22:48 +00:00
Copy link

"One of..."

"One of..."
Sign in to join this conversation.
Reviewers
No reviewers
LeSasse
fraimondo
Labels
Clear labels   
CRITICAL
High Priority Issue

  
Stale

   
WIP
Work in progress

  
bug
Something isn't working

  
concept
Design/Implementation concepts

  
coordinate
Issues or pull requests related to coordinates (ROIs)

  
dataset
Issues or pull requests related to datagrabbers or datasets

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
maintenance
Issues or pull requests related to maintenance

  
maps
Issues or pull requests related to maps (probabilistic atlases)

  
marker
Issues or pull requests related to markers

  
mask
Issues or pull requests related to masks

  
on hold
Issue waiting on other issue(s)

  
parcellation
Issues or pull requests related to parcellations (deterministic atlases)

  
preprocess
Issues or pull requests related to preprocessors

  
question
Further information is requested

  
ready
Pull request is ready for review and merge

  
storage
Issues or pull requests related to marker storage or retrieval

  
template-space
Issues or pull requests related to template spaces

  
triage
New issues waiting to be reviewed

  
wontfix
This will not be worked on

No labels
CRITICAL
Stale
WIP
bug
concept
coordinate
dataset
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
maintenance
maps
marker
mask
on hold
parcellation
preprocess
question
ready
storage
template-space
triage
wontfix
Milestone
Clear milestone
No items
No milestone
0.0.4 (alpha 3)
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
juaml/junifer!317
No description provided.