-
Notifications
You must be signed in to change notification settings - Fork 9
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
2024-2.3 envs #41
2024-2.3 envs #41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look generally good to me. I am not 100% sure about the CI failure, but this seems to be a relevant open issue on micromamba.
- maggma >=0.66 | ||
- mp-api >=0.41.2 | ||
- pychx >=4.3.1 | ||
- pycryptodome |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I am glad pycryptodome
is added in env-py3{10,11,12}.yml
I have a general question about this type of packages.
It seems pycryptodome
is already a dependency in nslsii
(https://github.com/NSLS-II/nslsii/blob/2a31a9950d1fc266ae212c5c64ab6f83bab447b0/requirements.txt#L18) but for whatever reason the conda env is not resolving to that version of nslsii
, so not picking up pycryptodome
. If newer packages are installed on an overlay do we generally expect these conda environments to carry the dependencies to support them, or should the dependencies be installed on the overlay itself?
If we allow some "future" dependencies out of necessity it would be good to keep a list of them somewhere. Just a suggestion.
Alternatively, should we pin nslsii
to a newer version instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we never went back to https://github.com/conda-forge/nslsii-feedstock/blob/main/recipe/meta.yaml and added the requirement. Please feel free to submit a PR there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seem to be few other dependencies missing in https://github.com/conda-forge/nslsii-feedstock/blob/main/recipe/meta.yaml. For example, httpx, msgpack, msgpack-numpy, packaging, redis-json-dict
are also missing. Should they also be added to feedstock? They are all available on conda-forge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's create an issue and address it in 2024-2.4
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good to me.
@@ -83,6 +83,39 @@ jobs: | |||
conda config --show | |||
printenv | sort | |||
- name: Add packages for py311 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice workaround! Glad it worked.
Pins
event-model
>=1.21 and addscookiecutter
.Closes #40.
Diffs: