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

Updated packages in Python environment #63

Merged
merged 4 commits into from
Feb 16, 2023
Merged

Updated packages in Python environment #63

merged 4 commits into from
Feb 16, 2023

Conversation

emiliom
Copy link
Member

@emiliom emiliom commented Feb 13, 2023

Added a few packages (contextily, folium, rioxarray, pip, and the pip package echopy), including two language packs (spanish & portuguese). Updated pangeo-notebook to 2023.02.11.

Tried to update conda-lock but ran into problem with pip dependencies.

See #42 (comment), oceanhackweek/Hub-Management#2 and oceanhackweek/Hub-Management#3

@emiliom emiliom requested a review from abkfenris February 13, 2023 03:27
@abkfenris
Copy link
Collaborator

Lets just go a cross post this: #42 (comment)

@emiliom It looks like echopy is a simple enough package to quickly get on Conda-Forge that grayskull probably can auto scaffold the recipe for staged-recipies, so that might be the best route currently, as I don't think conda can directly ingest the non-rendered lockfiles.

@ocefpaf only mamba and micromamba can use the intermediate lockfiles right?

I don't think I'm going to be able convert to a mamba/micromamba based image before this event, and if there are any issues the cross-language debugging might be messy so it's probably better not to try.

# PyPI package is older,
# but installing from github leads to conda-lock error
# - git+https://github.com/open-ocean-sounding/echopy.git
- echopy
Copy link
Member

Choose a reason for hiding this comment

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

This package does not install its dependencies and pip installing it will bring a broken pkg, or not fully functioning at best. I sent conda-forge/staged-recipes#22020 PR to help out a bit with its installation. It is only missing one optional dependency as far as I can tell.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!!!

@@ -94,3 +102,8 @@ dependencies:
- xgcm
- xlrd
- zarr
- pip:
# PyPI package is older,
Copy link
Member

Choose a reason for hiding this comment

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

According to the history in the upstream repo there are no functional changes in the actual package since the last PyPI release. You should be OK with it. See https://github.com/open-ocean-sounding/echopy/commits/master

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

@emiliom
Copy link
Member Author

emiliom commented Feb 15, 2023

How about if I remove echopy from this PR? That way it can move forward with the other updates, and come back with echopy later, when there might also be other package additions/updates.

@emiliom
Copy link
Member Author

emiliom commented Feb 15, 2023

@abkfenris and @ocefpaf The current Python image includes xlrd. IMHO it is useful to have Excel read capability. But xlrd is no longer maintained and doesn't read .xlsx files. Do you have any concerns about replacing it with openpyxl?

@abkfenris
Copy link
Collaborator

I don't have an issue swapping it out, especially since Pandas supports using it.

@emiliom
Copy link
Member Author

emiliom commented Feb 15, 2023

Thanks.

I forgot that openpyxl doesn't support reading .xls files, but xlrd does. I'm inclined to still drop xlrd altogether, given that it's getting old (though it doesn't seem to have any dependencies). So, I'll do that.

@ocefpaf
Copy link
Member

ocefpaf commented Feb 16, 2023

Do you have any concerns about replacing it with openpyxl?
I don't use xlrd anymore and we rarelly have non xlsx files. So I'm ok dropping it.

PS: I would also drop the conda-lock.yml file here b/c we only create the Linux lock anyway. You can do it with conda-lock-f environment.yml -p linux-64

- zarr
# - pip:
# - echopy
Copy link
Member

@ocefpaf ocefpaf Feb 16, 2023

Choose a reason for hiding this comment

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

This is in conda-forge now. You can try it out here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!!!

@emiliom
Copy link
Member Author

emiliom commented Feb 16, 2023

I've removed conda-lock.yml and updated the PR.

Regarding conda-lock-f environment.yml -p linux-64, I get an error "conda-lock-f: command not found". Maybe you meant conda-lock -f environment.yml -p linux-64? That's the statement I've used, per @abkfenris 's instructions in #42, followed by conda-lock render -p linux-64

@emiliom
Copy link
Member Author

emiliom commented Feb 16, 2023

Alright, I've re-created the conda environment (again with mamba) now with the new conda-forge echopy package, and updated the conda-lock file. This is now ready to go, hopefully!

@ocefpaf ocefpaf merged commit 082c693 into oceanhackweek:main Feb 16, 2023
@emiliom
Copy link
Member Author

emiliom commented Feb 16, 2023

Thanks!

@emiliom emiliom deleted the updpyenvconda branch February 20, 2023 07:36
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.

3 participants