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

Move fiona to be installed from conda forge #987

Merged
merged 2 commits into from
Feb 8, 2021

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Feb 5, 2021

fiona is again giving us headaches, the reason why I am moving it to be a conda-forge dependency instead of PyPi is in this comment as a result of @remi-kazeroni noticing fiona trying to read an old csv file from GDAL that has disappeared from GDAL>=3.1 and asking us about it in this comment, also an issue documented here Toblerity/Fiona#897 (towards the bottom)

@remi-kazeroni
Copy link
Contributor

I have just checked and this PR fixed this issue mentioned above. Thanks for looking into this @valeriupredoi 👍

@valeriupredoi
Copy link
Contributor Author

brilliant, cheers Remi! can you pls approve so we get it merged and cherrypicked in the release? 🍺

Copy link
Contributor

@remi-kazeroni remi-kazeroni left a comment

Choose a reason for hiding this comment

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

Thanks @valeriupredoi 👍

@valeriupredoi valeriupredoi merged commit 5e45a88 into master Feb 8, 2021
@valeriupredoi valeriupredoi deleted the move_fiona_to_conda_forge branch February 8, 2021 11:05
@@ -32,9 +32,6 @@
'install': [
'cf-units',
'dask[array]',
# fiona: 1.8.18/py39, they seem weary to build manylinux wheels
# so we may have to install from conda-forge in the future
'fiona',
Copy link
Member

@bouweandela bouweandela Feb 8, 2021

Choose a reason for hiding this comment

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

The list in setup.py lists all dependencies, regardless of how they're installed. Could you put fiona back here @valeriupredoi? If it's installed by conda already, pip won't try to install it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aha will do now!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#990 pls approve so we merge and release the fiona baby

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cheers for pointing out and reviewing, bud!

jvegreg pushed a commit that referenced this pull request Feb 9, 2021
* added fiona as dep

* removed fiona as pypi dep
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installation Installation problem release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants