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

Multiple documentation updates #948

Merged
merged 11 commits into from
Sep 22, 2023

Conversation

handwerkerd
Copy link
Member

@handwerkerd handwerkerd commented May 18, 2023

Closes #683. Closes #898. Closes #900. Closes #942

This will be a PR to address many of the open issues regarding documentation. As I address more, I'll update this initial message. Others are welcome to work on documentation issues/gap and add to this PR.

Changes proposed in this pull request:

@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (d11f021) 89.00% compared to head (fb39d0d) 89.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #948   +/-   ##
=======================================
  Coverage   89.00%   89.00%           
=======================================
  Files          27       27           
  Lines        3411     3411           
  Branches      622      622           
=======================================
  Hits         3036     3036           
  Misses        227      227           
  Partials      148      148           

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

@tsalo
Copy link
Member

tsalo commented Jun 15, 2023

@all-contributors please add @handwerkerd for code.

@allcontributors
Copy link
Contributor

@tsalo

I've put up a pull request to add @handwerkerd! 🎉

@handwerkerd handwerkerd marked this pull request as ready for review August 31, 2023 20:02
@handwerkerd
Copy link
Member Author

handwerkerd commented Aug 31, 2023

I think I've included all the open low-hanging-fruit documentation open issues. There are a few others that might take a bit more work and are sufficiently separate from the rest of these. It would be good to get reviews from both people who know what's happening under the hood and people who can see if I'm explaining things clearly. While you're reading, also keep an eye out for other simple things that could be written more clearly or have added info. @tsalo @eurunuela @dowdlelt @n-reddy @NaomiGaggi @marco7877

You can view the rendered documentation for this PR at: https://tedana--948.org.readthedocs.build/en/948/

tsalo
tsalo previously approved these changes Sep 21, 2023
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.

There might be a small typo, but LGTM otherwise.

docs/approach.rst Outdated Show resolved Hide resolved
Co-authored-by: Taylor Salo <tsalo90@gmail.com>
tsalo
tsalo previously approved these changes Sep 21, 2023
docs/multi-echo.rst Outdated Show resolved Hide resolved
docs/faq.rst Outdated
exclude voxels from some steps that have large dropout in later echoes. The adaptive
mask will flag some voxels in common dropout regions, but should not radically alter
the inputted mask. Here is more information on the
`creation and use of the adaptive mask`_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't recall - is the original input mask still used as the mask for the final data (even if the ICA is run on a reduced mask due to drop out).

Copy link
Member Author

Choose a reason for hiding this comment

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

All voxels with at least 1 good echo are included. The revised text clarifies this.

docs/outputs.rst Outdated Show resolved Hide resolved
docs/approach.rst Outdated Show resolved Hide resolved
docs/outputs.rst Outdated Show resolved Hide resolved
dowdlelt
dowdlelt previously approved these changes Sep 21, 2023
Copy link
Collaborator

@dowdlelt dowdlelt left a comment

Choose a reason for hiding this comment

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

github is being finicky...I approve pending typo corrections

@handwerkerd handwerkerd dismissed stale reviews from dowdlelt and tsalo via a540b3d September 21, 2023 21:14
a typo that my suggestion didn't fix
Copy link
Collaborator

@dowdlelt dowdlelt left a comment

Choose a reason for hiding this comment

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

I abused my powers to fix a typo, because honestly it feels like kind of insane to point at a typo, say I approve, get that dismissed, then approve again. I approve!

@handwerkerd handwerkerd merged commit efe7cdb into ME-ICA:main Sep 22, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants