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

Namespace packaging #1

Closed
oesteban opened this issue Apr 18, 2023 · 12 comments
Closed

Namespace packaging #1

oesteban opened this issue Apr 18, 2023 · 12 comments

Comments

@oesteban
Copy link
Member

shouldn't this:

name = "synthstrip"

be

 name = "nipreps-synthstrip"

as per https://packaging.python.org/en/latest/guides/packaging-namespace-packages/#native-namespace-packages?

And then also (https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#finding-namespace-packages):

[tool.setuptools.packages.find]
where = ["nipreps"]
@mgxd
Copy link
Contributor

mgxd commented Apr 18, 2023

Yep, the name should absolutely be nipreps-synthstrip, that was a remnant of my earlier fiddling.

Not sure about adding a setuptools command though, hatchling should handle this automatically I believe (cc our resident hatchling expert @effigies)

@oesteban
Copy link
Member Author

What about the other two submodules I proposed?

nipreps.interfaces
nipreps.workflows

@mgxd
Copy link
Contributor

mgxd commented Apr 18, 2023

I don't remember that proposal - are you suggesting the import should look like:

from nipreps.interfaces.synthstrip import SynthStrip

@effigies
Copy link
Member

Not sure about adding a setuptools command though, hatchling should handle this automatically I believe

Yes, hatchling and setuptools will (per spec) ignore each other's config. I think you're right that this should work (I would just build and unpack the sdist and wheel to verify) as-is, but if you want to be explicit, you will want the package to be nipreps and you might use only-include = ["nipreps/synthstrip"]. See https://hatch.pypa.io/latest/config/build/#explicit-selection.

@oesteban
Copy link
Member Author

I don't remember that proposal

Sorry, it's this - nipreps/nireports#71 (comment)

  • are you suggesting the import should look like:
from nipreps.interfaces.synthstrip import SynthStrip

I think so, for the interface. But to ease our lives we can put the nipype interface with the other interfaces, and leave this package even more lightweight.

@effigies
Copy link
Member

I would be up for trying to keep the interface and wrapped utility in the same package. We could even add a pydra task while we're at it.

nipreps/
  synthstrip/
  tasks/
    synthstrip/
  interfaces/
    synthstrip/

That way adapting the interface/task can be done in lockstep with changes to the utility and we can use PyPI don't need to depend on any versioning machinery in nipype or pydra.

I would keep nipype and pydra as optional dependencies.

@oesteban
Copy link
Member Author

This would be a good topic to discuss in the next IT - don't you think?

cc/ @esavary

@mgxd
Copy link
Contributor

mgxd commented Apr 18, 2023

My inclination is on keeping the namespace as lightweight as possible, and IMO nipreps.<tools> does enough to distinguish 1) what tool it is and 2) it is maintained by this organization.

I would keep nipype and pydra as optional dependencies.

+1 on anytime we can make the installation smaller.

What we could then do is create a separate cookiecutter (nipreps-submodules?) that creates the expected layout (building off chris's)

nipreps/
  synthstrip/
    <code>
    ...
    ext/
        nipype.py
        pydra.py

@effigies
Copy link
Member

Yeah, I'm fine with bundling it inside one namespace. I would just be explicit and call the submodule wrapper or wrappers:

from nipreps.<tool>.wrapper.nipype import <Interface>
from nipreps.<tool>.wrapper.pydra import <Task>

@oesteban
Copy link
Member Author

@mgxd can we finalize this? Having a pypi package would allow me to drop the code from mriqc and start using it elsewhere.

@mgxd
Copy link
Contributor

mgxd commented Aug 29, 2024

@oesteban done

https://pypi.org/project/nipreps-synthstrip/

@mgxd mgxd closed this as completed Aug 29, 2024
@oesteban
Copy link
Member Author

Thanks!

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

No branches or pull requests

3 participants