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.toml and lockfile #489

Closed
wants to merge 29 commits into from
Closed

add pixi.toml and lockfile #489

wants to merge 29 commits into from

Conversation

Jaapel
Copy link
Contributor

@Jaapel Jaapel commented Jul 15, 2024

Some projects are already working with pixi.
It is a nice environment manager for conda / mamba packages that is very fast in resolving and installing packages. For now I would like to use it to install the environment.

Did some testing and messing around to get pixi and submodules working together.

The new installation workflow using pixi is described in the readme, so please follow instructions there and let me know if you run into issues.

Should be merged at the same time as frontend-PR.

@Jaapel Jaapel marked this pull request as ready for review July 19, 2024 13:39
@Jaapel Jaapel self-assigned this Jul 19, 2024
@Jaapel Jaapel added the Feature New feature request label Jul 19, 2024
Copy link
Contributor

@savente93 savente93 left a comment

Choose a reason for hiding this comment

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

one commen, otherwise LGTM :D

pixi.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@LuukBlom LuukBlom left a comment

Choose a reason for hiding this comment

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

Looks logical, I do have some concerns about the pinning to commit hashes, but I assume there is a solution within pixi for that

pixi.toml Outdated Show resolved Hide resolved
@savente93
Copy link
Contributor

Looks logical, I do have some concerns about the pinning to commit hashes, but I assume there is a solution within pixi for that

Not sure about pinned commits, but in core we would just have a github action that would periodically update dependencies, I'm sure this can be made to work with the revisions as well, though I do also think we want to use branches instead of pinned commits no?

@savente93 savente93 linked an issue Jul 22, 2024 that may be closed by this pull request
3 tasks
@Jaapel
Copy link
Contributor Author

Jaapel commented Aug 15, 2024

You cannot refer a branch in pixi according to the options I find in the docs: https://pixi.sh/v0.27.1/reference/project_configuration/#git

So I pinned commits. I can change to the url #branchname that git understands, let me try it out.

@Jaapel Jaapel requested review from savente93 and LuukBlom August 15, 2024 09:58
Copy link
Contributor

@savente93 savente93 left a comment

Choose a reason for hiding this comment

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

:D

@savente93
Copy link
Contributor

@LuukBlom think we can merge? if so can you approve? because I did the merge with main I can't be the reviewer apparently

@LuukBlom
Copy link
Contributor

@savente93 I have some local changes for this branch that adds tasks to pixi.toml (tests/docs/buildexe, just pushed them now), and would rather merge when all tasks work as well.

Also, after looking into it, using a branch is not supported by pixi at the moment, so we need to pin to commit hashes unfortunately. see prefix-dev/pixi#1206

I remember you mentioning automatically updating, the hashes. Could finishing the tasks + automating this be something you'd like to pick up?

@savente93
Copy link
Contributor

That makes sense, I'm in favor of making sure it works well first as well, I just didn't realize there were things left.

I can look into updating the commit hashes, but I've also been working on removing the need to use a branch on our dependencies and making some good progress. So I think if you agree it's better to just wait until we don't have to use the branches anymore. Regardless I do agree we shouldn't merge this before we have either of those solutions

@LuukBlom
Copy link
Contributor

LuukBlom commented Oct 15, 2024

Actually @savente93, using a fresh install of pixi, cloning the repo, and then the task tests: pixi run tests is succesfull, as well as the user guide task: pixi run docs.

So I think it'd be good to just merge this to get pixi in there (and stop additional scope creep) and then open issues for updating the hashes automatically and getting the api docs (we dont even build this currently so this is a new addition) & build exe/installer tasks to work (currently done by 2 python scripts so should be doable, but also not required as they dont care if the env is pixi or conda).

Im testing it now on wsl to see if the tests are green there as well.

What do you say?

@savente93
Copy link
Contributor

If it works already, then I agree. I think it will be really nice to have this as a starting point

LuukBlom and others added 12 commits October 17, 2024 14:06
moved pixi config from pixi.toml to pyproject.toml.
added GUI to the pixi config.
Note:
Currently it assumes you to have the repos cloned next to eachother, in the future we could perhaps merge the gui code into this one so we dont have to juggle 2 repos (would also solve broken envs due to mismatching environments for the frontend and backend)
removed accidental workspace file
added .env to store and load mapboxtoken
@LuukBlom
Copy link
Contributor

Closing and reopening to hopefully let teamcity see this branch

@LuukBlom LuukBlom closed this Oct 30, 2024
@LuukBlom LuukBlom reopened this Oct 30, 2024
@LuukBlom
Copy link
Contributor

replaced by #567

@LuukBlom LuukBlom closed this Dec 18, 2024
@LuukBlom LuukBlom deleted the add_pixi branch December 20, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install via Pixi
3 participants