search

python-visualization / folium

Python Data. Leaflet.js Maps.
https://python-visualization.github.io/folium/
MIT License
6.93k stars 2.23k forks source link

Remove accidental Fiona dependency #1979

Closed hansthen closed 4 months ago

hansthen commented 5 months ago

Undo adding fiona to requirements.

Conengmo commented 4 months ago

I don't really understand, these changes have already been merged? See https://github.com/python-visualization/folium/commit/8bc6b02c2f8c13050cc2e7517e8e842a15711081, this is a commit to the main branch, so these changes are already there. I'd expect a PR that removes that line in requirements. Or a PR that reverts that commit, then adds only the wanted changes.

hansthen commented 4 months ago

@Conengmo I am getting mixed signals here. You say you do not understand, but from your suggestions I get the impression you do. Maybe I over interpret this, but I am left wondering if I did something that you are unhappy about.

Conengmo commented 4 months ago

Sorry if I’ve given offense! I didn’t mean to. I’m really a bit confused :) when I look at the ‘files changed’ I see changes that I think are already in the main branch. If I’m not mistaken https://github.com/python-visualization/folium/commit/8bc6b02c2f8c13050cc2e7517e8e842a15711081 is that commit that adds these changes to the main branch. So what is this PR supposed to do? I don’t mean that in a bad way, just trying to understand. Is there some git weirdness going on here?

hansthen commented 4 months ago

Sorry if I’ve given offense!

No offence taken; sometimes it is hard to communicate via text fields.

I made the change your requested on the already merged branch. However, I forgot to sync against the main repo first. I should have rebased it first or started from a clean branch.

Conengmo commented 4 months ago

That makes sense! Thanks for clarifying and for updating the branch.

Conengmo commented 4 months ago

I don’t know why that test failed but this can just be merged I suppose.