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

Choropleth and branca colormaps do not show up with numpy 2.0 #1905

Closed
martinfleis opened this issue Mar 20, 2024 · 10 comments · Fixed by python-visualization/branca#163
Closed

Comments

@martinfleis
Copy link
Collaborator

Installing numpy 2.0 into an environment from nightly wheels results in issues with folium.Choropleth and any of the branca colormaps (LinearColormap, StepColormap). They just won't show up on a map. At the same time, there's no error. Just an empty map.

I noticed this when geopandas CI started failing when install numpy 2.0 and now I confirmed manually that the only change in the environment that needs to happen is numpy. Stable is fine, nightly causes the issue. What puzzles me the most is that there's no error emitted.

You can check this env spec (https://github.com/geopandas/geopandas/actions/runs/8334414770/job/22807960989?pr=3042) for details on env but installing numpy nightly from https://pypi.anaconda.org/scientific-python-nightly-wheels/simple is enough to cause the trouble.

Easy to reproduce with the example from docs - https://python-visualization.github.io/folium/latest/getting_started.html#Choropleth-maps.

@Conengmo
Copy link
Member

Conengmo commented Apr 3, 2024

I haven't tested Numpy 2 myself yet, will do so. I do wonder though, if the stable release is fine, should we worry right now? Maybe something will get fixed before the problem with the nightly release gets into stable?

@martinfleis
Copy link
Collaborator Author

Numpy 2.0 comes with a lot of breaking changes so my bet is that whatever is causing this will not be fixed and is intentional. It may also be a bug but then we need to ensure they're aware of it.

@Conengmo
Copy link
Member

Conengmo commented Apr 3, 2024

I installed numpy==2.0.0rc1 and tried to verify compatibility.

For testing Choropleth I need Pandas, but there's not currently a Pandas version that's compatible with Numpy 2.0.0. It's expected to come out somewhere in the coming weeks. See pandas-dev/pandas#55519.

LinearColorMap seems to work without issue. Branca doesn't actually have a dependency on Numpy. Though it does check if Pandas is installed, so if Pandas is broken it will cause an error.

I'm now thinking we pin Folium to Numpy<2.0.0, and once our dependencies like Pandas are compatible, we make sure Folium also still works. List of status of dependencies: numpy/numpy#26191 How does that sound Martin?

Maybe Geopandas is a special case, since Folium uses it, but Geopandas also uses Folium. I guess when we can run Geopandas without issue on Numpy 2.0.0, even if it's not listed as officially compatible, we can drop the pin on Numpy.

@martinfleis
Copy link
Collaborator Author

Pandas nightly is compatible with numpy 2.0. Let me dig into this to see where it comes from. I don't think it is caused by any of the dependencies like pandas as the only change required in the environment composed of nightly versions is numpy.

@Conengmo
Copy link
Member

Conengmo commented Apr 3, 2024

I should say I didn't install from nightly sources, but from the 2.0.0 release candidate: https://github.com/numpy/numpy/releases/tag/v2.0.0rc1

@jakirkham
Copy link

Does Folium run CI with NumPy 2.0.0rc1?

@martinfleis
Copy link
Collaborator Author

@jakirkham no, though we probably should.

@jakirkham
Copy link

Thanks Martin! 🙏

Went ahead and filed issue: #1937

Please feel free to add any more context there

@ocefpaf
Copy link
Member

ocefpaf commented Apr 25, 2024

BTW, running CI with latest numpy may not help much we our tests are flaky and we use numpy in a very basic manner. Hence the blank page that would result in a passing CI :-/

We never created image checks or deep HTML inspection to catch something like this.

With that said, my guess is that np.histogram may have changed. I'll check the other issues/PRs opened.

@jakirkham
Copy link

Ah ok. Happy to defer to whatever you and Martin think is best 🙂

Thanks Filipe! 🙏

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 a pull request may close this issue.

4 participants