-
Notifications
You must be signed in to change notification settings - Fork 0
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
Set up conda package #2
Conversation
d0fb0bf
to
5c345dc
Compare
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.
Requesting changes on the paths that don't have .
in .conda
in the workflows. Outside of that, just some formatting things.
5c345dc
to
1e5905e
Compare
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.
Just one more that snuck through.
1e5905e
to
746d248
Compare
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.
Looks good. I don't think there is a pre-commit check running on this one, but if it needs similar formatting changes it should be a separate PR.
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.
Looks good, just a couple of minor changes.
keywords = ["ACCESS-RAM", "Regional Nesting Suite", "era5grib"] | ||
dynamic = ["version"] | ||
dependencies = [ | ||
"python >=3.10,<=3.12", |
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.
Oh right, yeah I think this should be a specific version. We're using 3.10
in payu
https://github.com/ACCESS-NRI/payu-condaenv/blob/main/env.yml#L7
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.
Yeah, this is not the right file for that.
This sets the python dependency for the era5grib_parallel
conda
package (similarly to what is done for the replace_landsurface).
The deployment environment is set in the access-ram-condaenv env.yaml, and we are also using python=3.10
(similar thing is done for the dev deployed environment).
I would personally update this to python=3.11
, but I have no strong feelings about it.
An important point is:
Why do we only specify the python
's version? Shouldn's we also specify the versions for the other dependencies there?
a322863
to
058565d
Compare
058565d
to
0eb2e0e
Compare
…ram-condaenv repo.
0eb2e0e
to
fb97e50
Compare
Closed as this PR has been substituted by #3. |
Fixes #1
I haven't touched the main scripts in this PR, and the new package has the
era5grib_parallel
entry point, to be called from the rose suite (to be changed in thenci_era5grib_parallel
taskrose-app.conf
file).