-
Notifications
You must be signed in to change notification settings - Fork 7
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: docker and dependency overhaul #47
Conversation
Hello @eilidhmacnicol! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2022-05-10 13:36:29 UTC |
96f7ead
to
b724200
Compare
bf95834
to
c13e911
Compare
092f47b
to
3f9234e
Compare
3f9234e
to
c3f1115
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I think we can save trouble in the long run by just dropping the tests where we call setup.py
directly. The officially recommended ways of doing things are to use python -m build
to create sdist
or wheel
, and pip install
to install anything.
Apologies for writing these at some point in the past; I thought it was being resilient, but it turns out to have just proved why the entire ecosystem is moving away from these strategies.
Totally agreed. Let's migrate asap |
Thanks both for the speedy reviews.
Is this the sort of change that I should wait for before tagging and updating Docker, or should I go ahead to see if it solves #46 ? |
It's not critical, packages seem to build just fine. But it is likely that is not going to be the case for long |
Changes proposed in this pull request
fixes #46 as well as resolving the dependency on niworkflows/feat/infants-rodents for the EPI reference workflow.
Depends on nipreps/nirodents#57 being merged for a new nirodents tag, then a new release can be made here to update the docker hub image.
Documentation that should be reviewed