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

Poetry #541

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Poetry #541

wants to merge 7 commits into from

Conversation

ctr26
Copy link

@ctr26 ctr26 commented Nov 16, 2023

Hi again,

Am I ok to add poetry dependency stuff to the pyproject toml?

Thanks

Craig

@github-actions github-actions bot added ci Continuous Integration documentation Improvements or additions to documentation github_actions Pull requests that update Github_actions code labels Nov 16, 2023
@ctr26 ctr26 requested review from FynnBe and oeway and removed request for FynnBe November 16, 2023 18:08
@FynnBe
Copy link
Member

FynnBe commented Nov 17, 2023

I haven't used poetry myself, so please keep that in mind...
Couple questions

  • benefits over mamba?
  • dependencies in yet another location? setup.py, environment.yaml... and now poetry? (how) can this be simplified?

stricter pinning which seems to be promoted by the use of poetry seems nice, but we don't want pin too strictly as this library should be easy to add to other envs.

@ctr26
Copy link
Author

ctr26 commented Nov 17, 2023

Poetry is mamba compatible, poetry just completely replaces the old setup.py stuff. setup.py and setuptools also aren't recommended by PEP anymore

So with this implementation you can run pip install . or pip install git+https://github.com/bioimage-io/spec-bioimage-io.git or people who dont want to use conda for instance

@ctr26
Copy link
Author

ctr26 commented Nov 17, 2023

Realistically your env file could just then be


channels:
- anaconda
- defaults
dependencies:
- pip
- pip:
  - -e .

@ctr26
Copy link
Author

ctr26 commented Nov 17, 2023

Poetry also has built in functionality for pushing and building straight to Pypy

@FynnBe
Copy link
Member

FynnBe commented Nov 18, 2023

ah, so we can remove setup.py then?
but it's not possible to install packages from conda channels using poetry?

@ctr26
Copy link
Author

ctr26 commented Nov 18, 2023

Exactly.

@ctr26
Copy link
Author

ctr26 commented Nov 18, 2023

As for the current error on ci, pip is supposed to work out of the box with pyproject tomls that have poetry in them, so I need to figure out why it's failing

@ctr26
Copy link
Author

ctr26 commented Nov 18, 2023

More generally I tend to only use conda for installing packages that don't work outright on pypi, like the cudatoolkit; which is usually compiled binaries I've found.

@jmetz
Copy link

jmetz commented Nov 18, 2023

Ahh, the good old dependency management fun-and-games 😅

I haven't used poetry extensively but my initial impressions are that it's reasonably good, and adding the option of using poetry seems like a good idea.

On a related note, I'm keeping an eye on rye (rye-up.com), as it seems like it has a lot of potential to solve a lot of the python installation/distribution/dependency management issues. That said, it could end up being just another anaconda/mamba etc etc... Though Armin's contributions to the python ecosystem have been pretty solid in the past, so I'm hopeful about this latest one.

@FynnBe
Copy link
Member

FynnBe commented Nov 19, 2023

still not 100% onboard, but doesn't look like it hurts, if you maintain it.
It shouldn't interfer with the cond build though: ModuleNotFoundError: No module named 'poetry' ... so if it could be separate. like an optional dependency that would be great.. or is that not feasible somehow??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration documentation Improvements or additions to documentation github_actions Pull requests that update Github_actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants