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

feat: Add pixi to the template #305

Merged
merged 43 commits into from
Oct 2, 2024
Merged

Conversation

YurelyCamacho
Copy link
Collaborator

Pull Request description

Edit and create files needed to add pixi to the template

Fixes #262

Pull Request checklists

This PR is a:

  • bug-fix
  • new feature
  • maintenance

About this PR:

  • it includes tests.
  • the tests are executed on CI.
  • the tests generate log file(s) (path).
  • pre-commit hooks were executed locally.
  • this PR requires a project documentation update.

@@ -115,6 +115,8 @@ groups:
maturin build
{%- elif cookiecutter.build_system == "pybind11" %}
python -m build
{%- elif cookiecutter.build_system == "pixi" %}
cargo build
Copy link
Member

Choose a reason for hiding this comment

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

My impression is that it should be pixi install

@@ -129,6 +129,8 @@ build:
maturin build
{%- elif cookiecutter.build_system == "pybind11" %}
python -m build
{%- elif cookiecutter.build_system == "pixi" %}
cargo build
Copy link
Member

Choose a reason for hiding this comment

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

Also here, i guess that it should be pixi install ..

@xmnlab
Copy link
Member

xmnlab commented Jul 20, 2024

https://github.com/osl-incubator/scicookie/actions/runs/10016410500/job/27689312914#step:5:5566

Not sure why it is not finding pyproject.toml .. maybe it is missing som pixi table in the pyproject.toml

@YurelyCamacho
Copy link
Collaborator Author

@xmnlab Can you help me with this please.

@xmnlab
Copy link
Member

xmnlab commented Jul 31, 2024

@YurelyCamacho , this is the error in the log:

/home/runner/work/scicookie/scicookie/tests/smoke/base.sh: line 102: ipython: command not found

that happens because pixi doesn't install the dependencies in the current environment, it creates an isolated environment for that.

so you need to activate the environment first or execute it with the pixi run command:

It is not encouraged to use the traditional conda activate-like activation, as deactivating the environment is not really possible.

so I would go with pixi run approach. In the past we used something similar for hatch, so you can use the same approach: 32704a5#diff-1c8f1585e21b07c45bdfaa5e9e131567e4c361fbae1a7fc2498a60ba061d70ddR43

@YurelyCamacho
Copy link
Collaborator Author

@xmnlab I used the approach you told me about in the previous comment and the same error remains. I also changed your suggestion of cargo build to pixi install and also python -m build and it didn't work. I made other changes based on hatch and it doesn't work either (for example, I changed pixi install to pip install “.[dev]” in the base.sh).

@YurelyCamacho
Copy link
Collaborator Author

@xmnlab I can't run the tests locally because I keep getting the error I told you before about mamba, and I have it installed in the environment

@xmnlab
Copy link
Member

xmnlab commented Jul 31, 2024

@xmnlab I can't run the tests locally because I keep getting the error I told you before about mamba, and I have it installed in the environment

let me know if you want a quick call to check that together.

about the current error on ci, the COMMAND_PREFIX is just to run the other commands with pixi run ... but you still need to install the dependencies .. so maybe with pip install should do the trick .. so you need to have both

@YurelyCamacho
Copy link
Collaborator Author

@xmnlab Yes, tomorrow would be good to verify that. What time can you do it?
I'll see if what you told me works, thanks.

@YurelyCamacho YurelyCamacho force-pushed the 262-add-pixi branch 2 times, most recently from 30fc8c2 to 93b9c2a Compare September 12, 2024 01:25
@YurelyCamacho
Copy link
Collaborator Author

@xmnlab I think the problem is in the [tool.pixi.dependencies]. I will leave the prefix.dev answer here.

Hey! The [pypi-dependencies] part doesn't do anything as for pixi to understand it you would need to add tool.pixi to it.

I see you added build to the dev optional dependencies. If you want to install a dev env with pixi you need to add:

[environments]
dev = ["dev"]

then you should be able to do pixi run -e dev python -m build

else
echo "Makim and Make were not found in the system."
exit 1

fi

python -c "import osl_python_package as mypkg; assert mypkg.__version__ == '0.1.0'"
$COMMAND_PREFIX python -c "import osl_python_package as mypkg; assert mypkg.__version__ == '0.1.0'"
Copy link
Member

Choose a reason for hiding this comment

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

not sure .. but it looks like that there is any issue running this with pixi ... so maybe you could try a different approach:

Suggested change
$COMMAND_PREFIX python -c "import osl_python_package as mypkg; assert mypkg.__version__ == '0.1.0'"
$COMMAND_PREFIX python -c 'import osl_python_package as mypkg; assert mypkg.__version__ == "0.1.0"'

@YurelyCamacho YurelyCamacho marked this pull request as ready for review October 2, 2024 01:31
@xmnlab
Copy link
Member

xmnlab commented Oct 2, 2024

LGTM! Good job, @YurelyCamacho ! Thanks for working on that

@xmnlab xmnlab merged commit f07e14c into osl-incubator:main Oct 2, 2024
19 checks passed
Copy link

🎉 This PR is included in version 0.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add Pixi package manager to the template
2 participants