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

Add pixi #798

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

Add pixi #798

wants to merge 2 commits into from

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented Jul 12, 2024

Checklist

  • Used a static YAML file for the patch if possible (instructions).
  • Only wrote code directly into generate_patch_json.py if absolutely necessary.
  • Ran pre-commit run -a and ensured all files pass the linting checks.
  • Ran python show_diff.py and posted the output as part of the PR.
  • Modifications won't affect packages built in the future.

@wolfv wolfv requested a review from a team as a code owner July 12, 2024 21:15
@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

@hmaarrfk
Copy link
Contributor

is a pixi lock file for a "small project" such as this really 5k lines long?

@wolfv
Copy link
Member Author

wolfv commented Sep 22, 2024

It will become a little shorter soon (we remove some duplicate data in a upcoming PR). But yeah, since we need the repodata records it is rather long.

@bollwyvl
Copy link
Contributor

I think the approach from staged-recipes of not checking in the pixi.lock is going to be the right play here. On account of that, the bottom dependency versions should be tightened a fair amount and kept in sync with the meta.yaml so that migrations trickle down into pixi.toml and trigger a re-lock locally as needed.

I could definitely see a useful pixi r pr top-level command that:

  • pixi r lint
    • validated all patches
  • pixi r diff
    • ran show_diff.py
      • wrote it out to $(git rev-parse --abbrev-ref HEAD).diff.md for easy of copy-pasting and review

Copy link
Member

@jdblischak jdblischak left a comment

Choose a reason for hiding this comment

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

Could you please also add some documentation to recipe/README.md to alert contributors that the pixi configuration exists and how they can use it to test their patches? I'm imagining something similar to #675 where I added a conda env file

show-diff = "python ./show_diff.py"

[dependencies]
annotated-types = "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

Already a dependency of pydantic...

conda-build = "*"
license-expression = "*"
pre-commit = "*"
pydantic = "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

I... don't believe this can be *... I'd wager 2.* might be safe, but there are a lot of pins out there...

platforms = ["osx-arm64", "linux-64", "win-64"]

[tasks]
show-diff = "python ./show_diff.py"
Copy link
Contributor

Choose a reason for hiding this comment

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

as this is:

  • very expensive, I recommend adding a description noting that
  • _very_verbose, probably also a task which writes it out to a > {git branch}.txt

annotated-types = "*"
conda-build = "*"
license-expression = "*"
pre-commit = "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

pre-commit isn't going to fire unless inside a shell... having a task.lint that runs all the linters against all the files would likely be helpful.

@bollwyvl
Copy link
Contributor

bollwyvl commented Jan 6, 2025

I'm not convinced checking in the pixi.lock is a net win for this particular project...

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.

4 participants