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

Move pymcubes to an optional dependency. #310

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

willGraham01
Copy link
Collaborator

@willGraham01 willGraham01 commented Jun 14, 2024

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

PyMCubes is a dependency that is only required during atlas generation, and currently doesn't have an ARM package on conda-forge.

Forcing it to be a core dependency results in failed builds when installing via conda-forge, and since it's not needed for registration, we have decided to move it to an optional atlasgen dependency that users will not obtain by default.

After this, we'll need a new PyPI release and then to fix the corresponding conda-forge feedstock PR that will be created.

What does this PR do?

Moves PyMCubes (and other altas-generation only) dependencies to an optional atlasgen dependency group.

References

brainglobe/brainreg#206

How has this PR been tested?

Local pip install -e ..

Is this a breaking change?

Does this PR require an update to the documentation?

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@willGraham01 willGraham01 requested a review from a team June 14, 2024 11:00
@adamltyson
Copy link
Member

Should we add other packages that aren't needed for the atlasAPI? Maybe ITK is one?

@willGraham01
Copy link
Collaborator Author

willGraham01 commented Jun 14, 2024

A quick search of this repo indicates that the following packages are only used in the atlas_generation/ submodule:

  • loguru
  • PyMCubes
  • SimpleITK
  • tqdm
  • vedo
  • xmltodict

Also, we seem to be fetching scikit-image but never actually importing it... so I guess it can just go?

Changes applied in 663bb21

Copy link
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

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

Looks good to me. Would you mind open an issue in the docs repo to update the "how to contribute an atlas" accordingly, please?

@alessandrofelder alessandrofelder merged commit 270b1fe into main Jun 14, 2024
13 checks passed
@willGraham01 willGraham01 deleted the wgraham-pymcubes-optional branch June 14, 2024 14:01
@willGraham01
Copy link
Collaborator Author

willGraham01 commented Jun 14, 2024

brainglobe/brainglobe.github.io#203

Will also publish a new release once the tests pass on main.

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