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

Maint: volume.fetch #562

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Maint: volume.fetch #562

merged 2 commits into from
Feb 9, 2024

Conversation

AhmetNSimsek
Copy link
Collaborator

During the last refactoring, this method has been changed a few times, leaving some artifacts and small issue to resolve. This PR addresses these.

  • Clean up the code
  • Repeat only in case of too many requests
  • Check if "resolution_mm" supplied without format and set to NG (this behaviour already existent in parcellationmap.Map.fetch() but this is going to be removed as it is not it's job.) (Alternatively, the fetch can fail at, say NiftiProvider.fetch() but it is not user's job to know which formats are shipped with different resolutions. And since we decided against resampling nifti images if resolution_mm is supplied, this seems to be the best solution at the moment.)
  • formats property should only list actual formats otherwise recursion occurs.

- Clean up the code
- Repeat only in case of too many requests
- Check if "resolution_mm" supplied without format
@AhmetNSimsek AhmetNSimsek added the maintenance Not a bug or breaking issue. Code maintenance related. label Feb 8, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: 1305 lines in your changes are missing coverage. Please review.

Comparison is base (5a0e6b3) 36.81% compared to head (3ddcb1e) 53.29%.
Report is 387 commits behind head on main.

Files Patch % Lines
siibra/features/feature.py 48.55% 125 Missing ⚠️
siibra/volumes/volume.py 43.89% 124 Missing ⚠️
siibra/core/region.py 56.75% 112 Missing ⚠️
siibra/volumes/parcellationmap.py 18.97% 111 Missing ⚠️
siibra/explorer/url.py 0.00% 107 Missing ⚠️
...bra/features/connectivity/regional_connectivity.py 33.33% 56 Missing ⚠️
siibra/retrieval/datasets.py 49.53% 54 Missing ⚠️
siibra/explorer/util.py 0.00% 51 Missing ⚠️
...a/features/tabular/receptor_density_fingerprint.py 4.54% 42 Missing ⚠️
...a/features/tabular/regional_timeseries_activity.py 39.13% 42 Missing ⚠️
... and 36 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #562       +/-   ##
===========================================
+ Coverage   36.81%   53.29%   +16.47%     
===========================================
  Files          61       69        +8     
  Lines        5421     6638     +1217     
===========================================
+ Hits         1996     3538     +1542     
+ Misses       3425     3100      -325     

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

@AhmetNSimsek AhmetNSimsek merged commit 75924f3 into main Feb 9, 2024
27 checks passed
@AhmetNSimsek AhmetNSimsek deleted the maint_volume_fetch branch February 9, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Not a bug or breaking issue. Code maintenance related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants