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

2024-2.3 envs #41

Merged
merged 9 commits into from
Jul 31, 2024
Merged

2024-2.3 envs #41

merged 9 commits into from
Jul 31, 2024

Conversation

mrakitin
Copy link
Contributor

@mrakitin mrakitin commented Jul 17, 2024

Pins event-model >=1.21 and adds cookiecutter.

Closes #40.

Diffs:

diff envs/env-py311.yml envs/env-py312.yml
1c1
< name: 2024-2.3-py311-tiled
---
> name: 2024-2.3-py312-tiled
10c10
<   - python >=3.11,<3.12
---
>   - python >=3.12,<3.13
14c14
<   - ansiwrap
---
>   # - ansiwrap
48c48
<   - diffpy.pdfgui
---
>   # - diffpy.pdfgui  # no package for py312
131c131
<   - pyfftw
---
>   # - pyfftw  # no build for py312 as of 2024-06-20
187c187
<   - shadow3 >=23.1.4
---
>   # - shadow3 >=23.1.4
189c189
<   - sirepo-bluesky >=0.6.2
---
>   # - sirepo-bluesky >=0.6.2
194a195
>     - ansiwrap
195a197
>     - bloptools >=0.7.0
198c200
<     - ophyd-async[ca,pva]
---
>     # - ophyd-async[ca]
206d207
<   - bloptools >=0.7.0
236c237
<   - ortools-python
---
>   # - ortools-python

@mrakitin mrakitin marked this pull request as ready for review July 17, 2024 19:44
@mrakitin mrakitin marked this pull request as draft July 18, 2024 20:09
@mrakitin mrakitin marked this pull request as ready for review July 19, 2024 18:09
@mrakitin mrakitin marked this pull request as draft July 19, 2024 21:04
Copy link
Contributor

@hyperrealist hyperrealist left a 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
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

@mrakitin mrakitin marked this pull request as ready for review July 31, 2024 19:59
Copy link
Contributor

@hyperrealist hyperrealist left a 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
Copy link
Contributor

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.

@hyperrealist hyperrealist merged commit 76bb141 into main Jul 31, 2024
3 checks passed
@hyperrealist hyperrealist deleted the 2024-2.3 branch July 31, 2024 23:21
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.

Requirements for 2024-2.3 conda envs
2 participants