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

Support Poetry version syntax and optional dependencies #454

Merged
merged 8 commits into from
Mar 27, 2023

Conversation

dlqqq
Copy link
Contributor

@dlqqq dlqqq commented Mar 22, 2023

@marcelotrevisani Thank you for all of your contributions! This is a critical project to the Conda Forge ecosystem, and I noticed that you have been maintaining this for 3+ years. I figured I should pay back your generosity with my own. I would appreciate your review on this PR. 🤗 ❤️

Description

This PR implements support for the most common Poetry projects by correctly handling Poetry version syntax and optional dependencies.

What this PR implements:

  • Correct encoding of Poetry dependency versions including support for the Poetry-exclusive ^ and ~ version operators
  • Fixes bug where Poetry dependency specification fields like optional were being included in the Conda recipe version specifiers
  • Support for Poetry optional dependencies declared via optional = true
  • Implements additional unit and snapshot tests for Poetry projects

What this PR does NOT implement:

  • Guaranteed correct handling of pre/post/dev release segments like 1.2.3a1
  • Support for Poetry Python restricted dependencies declared via python = "..."
  • Support for all other Poetry dependency fields like: git, path, url, source, markers

Strategy

Version specifiers

Poetry version operators are mostly compatible with Conda recipe version operators, with the exception of ^ and ~ which need to have custom encoding logic. The expected behavior is roughly:

  • ^M.m.p>=M.m.p,<(M+1).0.0
  • ~M.m.p>=M.m.p,<M.(m+1).0

(format: Poetry version syntax ⇒ Conda recipe version syntax)

As you might expect, this logic is a bit trickier because Poetry also allows non-semver version specifiers like ~1 or ^0, and has special edge cases for major version 0. Please see the implementation in py_toml.py for details.

Optional dependencies

Optional dependencies need to be listed under requirements.run_constrained in the recipe metadata.

Complete example

The Poetry dependency:

tensorflow-text = {version = "^2.11.0", optional = true, python = "^3.10, <3.12"}

Should be listed in the recipe metadata as:

requirements:
  run_constrained:
    tensorflow-text >=2.11.0,<3.0.0

We are implicitly ignoring all Poetry dependency specification fields aside from version and optional at the moment, as it’s not clear how to support them.

Suggested next steps

  • Review, iterate, and merge this PR.
  • Cut a new 2.3.0 release.
  • Investigate how to support Python restricted dependencies.
  • Perform a refactor of the code handling Conda recipe metadata synthesis. Let me explain my reasoning:
    • The source code is overly imperative, especially for the pyproject case. I had a lot of difficulty understanding the code because each metadata field was being determined manually through a long chain of dict lookups, dict merges, and conditional if statements. This style of code is brittle and prone to failure.
    • For example, the pyproject metadata gets merged manually with the setup.py/setup.cfg metadata, which then gets merged with PyPi metadata. This mysterious, Frankenstein "metadata" dictionary then finally gets transformed into Conda recipe metadata. This strategy will grow to be too difficult to maintain.
    • Eventually, I think the way forward is to have a CondaMetadata class that accepts a list of metadata sources and handles field resolution (i.e. “generate the list requirements.run, merging all dependencies together”) declaratively through a clean API.
    • @marcelotrevisani I can elaborate on this further in a future design document, if you are interested.

References

@dlqqq dlqqq requested a review from a team as a code owner March 22, 2023 22:10
@marcelotrevisani
Copy link
Member

Hi @dlqqq , thanks a lot for your contribution! I am happy that you are enjoying grayskull.

I have a few suggestions that I will add later today, I am at my day job, so I don't have much time to review right now, but I will do it later. Thank you!

🚀

@marcelotrevisani
Copy link
Member

can you add semver to this file please?
https://github.com/conda/grayskull/blob/main/environment.yaml

@dlqqq
Copy link
Contributor Author

dlqqq commented Mar 23, 2023

@marcelotrevisani Done. 😎

@marcelotrevisani
Copy link
Member

I have one more suggestion; after that, it should be good to go! :)

Thanks a lot for your contribution!

@dlqqq
Copy link
Contributor Author

dlqqq commented Mar 27, 2023

@marcelotrevisani Addressed your comments. 😁

@marcelotrevisani marcelotrevisani merged commit 413b1a1 into conda:main Mar 27, 2023
@marcelotrevisani
Copy link
Member

Thanks for your contributions! 🚀

@dlqqq
Copy link
Contributor Author

dlqqq commented Mar 27, 2023

@marcelotrevisani NP, thanks for reviewing. Do you think you could a 2.3.0 release soon so we can let people know that this issue is fixed on the latest version?

@marcelotrevisani
Copy link
Member

sure, let me cut a new release

@xylar
Copy link
Contributor

xylar commented Mar 27, 2023

@dlqqq, I really appreciate this work. The lack of pyproject.toml was becoming an increasing problem for me because I use grayskull as part of my workflow in maintaining a lot fo feedstocks.

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.

it is not handling ~ in dependencies [BUG] Optional pyproject deps have optional added to the version
3 participants