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

[UTILS] New utils function to compute voxel size for a volume #101

Closed
wants to merge 2 commits into from

Conversation

elodiegermani
Copy link
Collaborator

This Pull Request is related to issue [#](add a link to the issue here).

Before reviewing this Pull Request, please review Pull Request [#](add a link to the Pull Request here).

Changes proposed in this Pull Request:

  • Add a utils function get_vox_dims that can be used in pre-processing pipelines to get the size of voxels.
  • Don't really know where to add the documentation?

Checklist:

  • Descriptive title
  • Targets the main branch
  • Changes are functional
  • My code is explicit and comments were added to it
  • Code conforms with PEP8
  • Tests were added for the changes and they complete successfully
  • Existing tests were updated (if needed) and they complete successfully
  • Documentation was updated

@bclenet
Copy link
Collaborator

bclenet commented Sep 22, 2023

@elodiegermani, we could use this opportunity to start a new module containing "core" methods (i.e. methods common to almost all pipelines and dealing with their logic / neuro-imaging aspects). Although I have no idea how to name it for now and where it should be (maybe narps_open.core ?).

I know this is not documented enough for now but perhaps we should use narps_open.utils as place to store more general-purpose utilities.

I'll think about it and let you know. Any suggestions are welcome :)

To answer your question about documentation : the doc of this new module could be started in a new file under docs/.

@bclenet
Copy link
Collaborator

bclenet commented Nov 16, 2023

Hi @elodiegermani !

I started a new PR that contains the code you needed (get_vox_dims).
Once its is merged, you will be able to use the function in pipelines.

If you have time, please have a look to the PR to tell me what you think. Thanks ! 🙂

@bclenet
Copy link
Collaborator

bclenet commented Jan 30, 2024

Hi !
The PR is going to be closed because its code was added in the main branch, by merging PR #128 .

@bclenet bclenet closed this Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants