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

Add gerrychain recipe #7871

Merged
merged 10 commits into from
Mar 20, 2019
Merged

Add gerrychain recipe #7871

merged 10 commits into from
Mar 20, 2019

Conversation

maxhully
Copy link
Contributor

This PR adds a recipe for gerrychain, a Python library for mathematical research on gerrymandering and redistricting.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/gerrychain) and found some lint.

Here's what I've got...

For recipes/gerrychain:

  • There are too few lines. There should be one empty line at the end of the file.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/gerrychain) and found it was in an excellent condition.

requirements:
host:
- geopandas
- matplotlib
Copy link
Member

Choose a reason for hiding this comment

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

Are these packages really necessary at install time? If so, it might be good to look at your package's structure to try to make your imports not happen when calling setup.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out---I don't think they are necessary.

@msarahan
Copy link
Member

I'm glad to see this here, @maxhully. Thanks for keeping the project going so well. If you need help sorting out the install-time deps, let me know.

@msarahan
Copy link
Member

The mix of packages that is getting picked up here is really strange. I don't know exactly what the state of the geospatial stack on conda-forge is right now, but I'll try to explain why you have really weird stuff like util-linux coming in.

recipes/gerrychain/meta.yaml Outdated Show resolved Hide resolved
recipes/gerrychain/meta.yaml Outdated Show resolved Hide resolved
@msarahan
Copy link
Member

@ocefpaf are you aware of any strange incompatibilities in the geospatial stack right now? This recipe seriously goes sideways and pulls in ancient crap from the free channel. If I leave conda-forge off my channel list and only install from defaults, it comes out fine. So it's one or more of the conda-forge packages that have problems, but this stack is so deep that I rue having to figure out which dep exactly is causing it.


about:
home: https://github.com/mggg/GerryChain
license: BSD
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it is BSD-3-Clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Just pushed the fix.

@ocefpaf
Copy link
Member

ocefpaf commented Feb 27, 2019

@ocefpaf are you aware of any strange incompatibilities in the geospatial stack right now?

Nope. But it is not unlikely that we have old packages with bad metadata and/or something is not playing nice with defaults.

This recipe seriously goes sideways and pulls in ancient crap from the free channel. If I leave conda-forge off my channel list and only install from defaults, it comes out fine.

I can install the required packages here with only conda-forge just fine too.

So it's one or more of the conda-forge packages that have problems, but this stack is so deep that I rue having to figure out which dep exactly is causing it.

I'm guess we need to bite the bullet and identify the culprit to remove it 😄

I'll take a closer look tomorrow morning.

- pip
- python >=3.6
run:
- geopandas
Copy link
Member

Choose a reason for hiding this comment

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

geopandas is broken now b/c, even though the conda-forge stack has gdal pinned to libgdal 2.4.* a geopandas install pulls libgdal 2.3 from defaults. The staged recipes solution could be to use the strict channel option (ping @conda-forge/core) or maybe "wait out" the openssl migration and hopefully things will magically work themselves out (until the next migration).

In geopandas we can:

  • drop the optional dependency on psycopg2, which is causing the libgdal downgrade
  • over-specify libgdal 2.4 to force conda to do the right thing.

Ping @jorisvandenbossche.

Copy link
Member

Choose a reason for hiding this comment

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

Even with the new geopandas that pull less dependencies this still solves for libgdal 2.3.3 from defaults 😒

See conda-forge/osmnx-feedstock#40 (comment) for more info.

I'm not sure if we have any other option besides forcing the strict channel option in conda-forge.

Copy link
Member

Choose a reason for hiding this comment

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

I think that is reasonable to force strict, especially as that has been what we've told other people to do.

Copy link
Member

Choose a reason for hiding this comment

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

I'll send a PR to conda-smithy and staged-recipes tomorrow to implement that. I "forced" it here on CircleCI just as a test.

Copy link
Member

Choose a reason for hiding this comment

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

@maxhully we can force libgdal 2.4 here to "get this merged" b/c over-specifying the geopandas dependencies will force the right deps. Or we can wait for conda-forge/conda-forge-ci-setup-feedstock#50. The best solution is of course the latter but if you are in a hurry to get this package in we can do the former.

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'm OK with waiting! Let me know if there's anything I can do to help out, and thanks for all your work on this---both my recipe and conda-forge in general!

@ocefpaf ocefpaf requested a review from msarahan March 20, 2019 15:30
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/gerrychain) and found some lint.

Here's what I've got...

For recipes/gerrychain:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@ocefpaf
Copy link
Member

ocefpaf commented Mar 20, 2019

This is passing now. @maxhully and @msarahan are we OK to merge this one?

@maxhully
Copy link
Contributor Author

OK with me!

@ocefpaf ocefpaf merged commit 7edd68e into conda-forge:master Mar 20, 2019
PertuyF pushed a commit to PertuyF/staged-recipes that referenced this pull request Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants