juaml/junifer

Watch

Fork 0

[ENH]: Support for `Power 2013` coordinates #245

Merged

synchon merged 5 commits from feature/power-2013 into main

2023-10-04 15:49:31 +00:00

synchon commented

2023-08-10 08:04:09 +00:00

(Migrated from github.com)

Are you requiring a new dataset or marker?

I understand this is not a marker or dataset request

Which feature do you want to include?

We don't yet have the Power 2013 coordinates in-built. This will be a great addition.

How do you imagine this integrated in junifer?

As other in-built coordinates.

Do you have a sample code that implements this outside of junifer?

No response

Anything else to say?

No response

### Are you requiring a new dataset or marker? - [X] I understand this is not a marker or dataset request ### Which feature do you want to include? We don't yet have the [Power 2013](https://www.jonathanpower.net/2013-neuron-hubs.html) coordinates in-built. This will be a great addition. ### How do you imagine this integrated in junifer? As other in-built coordinates. ### Do you have a sample code that implements this outside of junifer? _No response_ ### Anything else to say? _No response_

codecov[bot] commented

2023-08-10 08:05:43 +00:00

(Migrated from github.com)

Codecov Report

Merging #245 (fbe18bc) into main (e3e9423) will decrease coverage by 0.06%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main     #245      +/-   ##
==========================================
- Coverage   93.10%   93.05%   -0.06%     
==========================================
  Files          84       84              
  Lines        3714     3716       +2     
  Branches      722      723       +1     
==========================================
  Hits         3458     3458              
- Misses        160      161       +1     
- Partials       96       97       +1

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`
junifer	`93.05% <33.33%> (-0.06%)`	⬇️

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

Files Changed	Coverage Δ
junifer/data/coordinates.py	`95.34% <33.33%> (-4.66%)`	⬇️

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/245?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#245](https://app.codecov.io/gh/juaml/junifer/pull/245?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (fbe18bc) into [main](https://app.codecov.io/gh/juaml/junifer/commit/e3e942324b2f90f103a8500105e123d664139737?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (e3e9423) will **decrease** coverage by `0.06%`. > The diff coverage is `33.33%`. [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/245/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/245?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #245 +/- ## ========================================== - Coverage 93.10% 93.05% -0.06% ========================================== Files 84 84 Lines 3714 3716 +2 Branches 722 723 +1 ========================================== Hits 3458 3458 - Misses 160 161 +1 - Partials 96 97 +1 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/245/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/245/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `100.00% <ø> (ø)` | | | [junifer](https://app.codecov.io/gh/juaml/junifer/pull/245/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `93.05% <33.33%> (-0.06%)` | :arrow_down: | 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. | [Files Changed](https://app.codecov.io/gh/juaml/junifer/pull/245?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/data/coordinates.py](https://app.codecov.io/gh/juaml/junifer/pull/245?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhL2Nvb3JkaW5hdGVzLnB5) | `95.34% <33.33%> (-4.66%)` | :arrow_down: |

github-actions[bot] commented

2023-08-10 08:11:33 +00:00

(Migrated from github.com)

PR Preview Action v1.4.4
🚀 Deployed preview to https://juaml.github.io/junifer/pr-preview/pr-245/
on branch `gh-pages` at 2023-09-06 09:39 UTC

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.4 :---: :rocket: Deployed preview to https://juaml.github.io/junifer/pr-preview/pr-245/ on branch [`gh-pages`](https://github.com/juaml/junifer/tree/gh-pages) at 2023-09-06 09:39 UTC

fraimondo (Migrated from github.com) requested changes

2023-09-05 12:08:00 +00:00

fraimondo (Migrated from github.com) left a comment

This is a backward's incompatible change. What about a deprecation cycle?

If I remember correctly, we can use functions to "load" coordinates/masks. Or we can implement it in a way that when someone uses "Power" it triggers a deprectation warning during a few release cycles. (We need to define).

This is a backward's incompatible change. What about a deprecation cycle? If I remember correctly, we can use functions to "load" coordinates/masks. Or we can implement it in a way that when someone uses "Power" it triggers a deprectation warning during a few release cycles. (We need to define).

synchon commented

2023-09-06 09:25:08 +00:00

(Migrated from github.com)

This is a backward's incompatible change. What about a deprecation cycle?

That is true. I went with the breaking change as we are still not v1.0.0 . But, I don't mind putting a deprecation notice.

If I remember correctly, we can use functions to "load" coordinates/masks. Or we can implement it in a way that when someone uses "Power" it triggers a deprectation warning during a few release cycles. (We need to define).

So, I'll put a deprecation notice stating that we'll remove it in the next release.

> This is a backward's incompatible change. What about a deprecation cycle? That is true. I went with the breaking change as we are still not v1.0.0 . But, I don't mind putting a deprecation notice. > If I remember correctly, we can use functions to "load" coordinates/masks. Or we can implement it in a way that when someone uses "Power" it triggers a deprectation warning during a few release cycles. (We need to define). So, I'll put a deprecation notice stating that we'll remove it in the next release.

fraimondo (Migrated from github.com) reviewed

2023-09-06 10:02:05 +00:00

junifer/data/coordinates.py

I was thinking to use something like this: https://deprecation.readthedocs.io/en/latest/

fraimondo (Migrated from github.com) reviewed

2023-09-06 10:03:31 +00:00

junifer/data/coordinates.py

while encapsulating the "power" in a function so we can keep track of the deprecation cycle. Otherwise we need to memorise stuff.

synchon (Migrated from github.com) reviewed

2023-09-06 10:20:56 +00:00

junifer/data/coordinates.py

This would require us to include another dependency for junifer and for one release cycle I don't think it's worth it. I think this makes sense once we start having objects which require backwards incompatible changes and that would happen after v1.0.0 IMO.