Markers/tsnr #171

Merged
LeSasse merged 17 commits from markers/tsnr into main 2023-03-08 10:51:58 +00:00
LeSasse commented 2023-01-05 14:51:12 +00:00 (Migrated from github.com)
  • fix #163
  • description of feature/fix
  • tests added/passed
  • add an entry to the latest changes

Add a sub-package to compute voxelwise, parcelwise, and spherewise temporal signal-to-noise ratio.

* [x] fix #163 * [x] description of feature/fix * [x] tests added/passed * [x] add an entry to the [latest changes](../docs/changes/latest.inc) Add a sub-package to compute voxelwise, parcelwise, and spherewise temporal signal-to-noise ratio.
codecov[bot] commented 2023-01-05 14:53:32 +00:00 (Migrated from github.com)

Codecov Report

Merging #171 (1fdcb8c) into main (7eda55b) will increase coverage by 0.08%.
The diff coverage is 98.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #171      +/-   ##
==========================================
+ Coverage   93.53%   93.62%   +0.08%     
==========================================
  Files          75       79       +4     
  Lines        2924     2980      +56     
  Branches      535      538       +3     
==========================================
+ Hits         2735     2790      +55     
- Misses        127      128       +1     
  Partials       62       62              
Flag Coverage Δ
docs 100.00% <ø> (ø)
junifer 93.61% <98.21%> (+0.08%) ⬆️

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

Impacted Files Coverage Δ
junifer/markers/temporal_snr/temporal_snr_base.py 96.00% <96.00%> (ø)
junifer/markers/__init__.py 100.00% <100.00%> (ø)
junifer/markers/temporal_snr/__init__.py 100.00% <100.00%> (ø)
...nifer/markers/temporal_snr/temporal_snr_parcels.py 100.00% <100.00%> (ø)
...nifer/markers/temporal_snr/temporal_snr_spheres.py 100.00% <100.00%> (ø)
# [Codecov](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#171](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (1fdcb8c) into [main](https://codecov.io/gh/juaml/junifer/commit/7eda55bab8da0f666f35fbaedec6334199dbcdb2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (7eda55b) will **increase** coverage by `0.08%`. > The diff coverage is `98.21%`. [![Impacted file tree graph](https://codecov.io/gh/juaml/junifer/pull/171/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://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #171 +/- ## ========================================== + Coverage 93.53% 93.62% +0.08% ========================================== Files 75 79 +4 Lines 2924 2980 +56 Branches 535 538 +3 ========================================== + Hits 2735 2790 +55 - Misses 127 128 +1 Partials 62 62 ``` | Flag | Coverage Δ | | |---|---|---| | docs | `100.00% <ø> (ø)` | | | junifer | `93.61% <98.21%> (+0.08%)` | :arrow_up: | 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. | [Impacted Files](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/markers/temporal\_snr/temporal\_snr\_base.py](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3RlbXBvcmFsX3Nuci90ZW1wb3JhbF9zbnJfYmFzZS5weQ==) | `96.00% <96.00%> (ø)` | | | [junifer/markers/\_\_init\_\_.py](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | | | [junifer/markers/temporal\_snr/\_\_init\_\_.py](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3RlbXBvcmFsX3Nuci9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | | | [...nifer/markers/temporal\_snr/temporal\_snr\_parcels.py](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3RlbXBvcmFsX3Nuci90ZW1wb3JhbF9zbnJfcGFyY2Vscy5weQ==) | `100.00% <100.00%> (ø)` | | | [...nifer/markers/temporal\_snr/temporal\_snr\_spheres.py](https://codecov.io/gh/juaml/junifer/pull/171?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3RlbXBvcmFsX3Nuci90ZW1wb3JhbF9zbnJfc3BoZXJlcy5weQ==) | `100.00% <100.00%> (ø)` | |
synchon commented 2023-01-05 15:01:26 +00:00 (Migrated from github.com)

Waits in queue until #107 and #169 are merged.

Waits in queue until #107 and #169 are merged.
github-actions[bot] commented 2023-02-28 14:52:10 +00:00 (Migrated from github.com)
PR Preview Action v1.3.0
Preview removed because the pull request was closed.
2023-03-08 10:56 UTC
[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.3.0 :---: Preview removed because the pull request was closed. 2023-03-08 10:56 UTC <!-- Sticky Pull Request Commentpr-preview -->
fraimondo (Migrated from github.com) requested changes 2023-03-03 14:24:10 +00:00
@ -0,0 +1,128 @@
"""Provide abstract base class for temporal signal-to-noise ratio (tSNR)."""
fraimondo (Migrated from github.com) commented 2023-03-03 14:22:45 +00:00

why is this in utils? Isn't it only from this method?

why is this in utils? Isn't it only from this method?
@ -0,0 +1,89 @@
"""Provide class for temporal SNR using parcels."""
fraimondo (Migrated from github.com) commented 2023-03-03 14:23:10 +00:00

outdated definition of masks.

outdated definition of masks.
fraimondo (Migrated from github.com) commented 2023-03-03 14:23:57 +00:00

no need to have this in utils, it can be in the base marker, making the TNSR marker self-contained.

no need to have this in utils, it can be in the base marker, making the TNSR marker self-contained.
synchon (Migrated from github.com) reviewed 2023-03-03 14:51:17 +00:00
@ -0,0 +1,128 @@
"""Provide abstract base class for temporal signal-to-noise ratio (tSNR)."""
synchon (Migrated from github.com) commented 2023-03-03 14:51:16 +00:00

It seemed like the place to keep when the PR was started, now of course, it makes sense to keep it in its own submodule.

It seemed like the place to keep when the PR was started, now of course, it makes sense to keep it in its own submodule.
synchon (Migrated from github.com) reviewed 2023-03-03 14:53:29 +00:00
@ -0,0 +1,89 @@
"""Provide class for temporal SNR using parcels."""
synchon (Migrated from github.com) commented 2023-03-03 14:53:29 +00:00

Oops, missed it.

Oops, missed it.
synchon (Migrated from github.com) reviewed 2023-03-03 14:55:46 +00:00
@ -0,0 +1,89 @@
"""Provide class for temporal SNR using parcels."""
synchon (Migrated from github.com) commented 2023-03-03 14:55:46 +00:00

Resolved in d94314b5.

Resolved in `d94314b5`.
synchon (Migrated from github.com) reviewed 2023-03-03 15:20:08 +00:00
@ -0,0 +1,128 @@
"""Provide abstract base class for temporal signal-to-noise ratio (tSNR)."""
synchon (Migrated from github.com) commented 2023-03-03 15:20:08 +00:00

Updated.

Updated.
synchon (Migrated from github.com) reviewed 2023-03-03 15:20:16 +00:00
synchon (Migrated from github.com) commented 2023-03-03 15:20:16 +00:00

Updated.

Updated.
fraimondo (Migrated from github.com) requested changes 2023-03-08 10:18:17 +00:00
fraimondo (Migrated from github.com) left a comment
Missing update in the docs: https://juaml.github.io/junifer/pr-preview/pr-171/builtin.html
synchon commented 2023-03-08 10:24:17 +00:00 (Migrated from github.com)
> Missing update in the docs: https://juaml.github.io/junifer/pr-preview/pr-171/builtin.html Done now.
fraimondo (Migrated from github.com) approved these changes 2023-03-08 10:47:58 +00:00
fraimondo commented 2023-03-08 10:48:06 +00:00 (Migrated from github.com)

LGTM!

LGTM!
Sign in to join this conversation.
No reviewers
No milestone
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!171
No description provided.