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

IndexError: pop index out of range when instanciating Topology with a list of GeoDataFrame #203

Closed
aspyk opened this issue Sep 29, 2023 · 10 comments

Comments

@aspyk
Copy link
Contributor

aspyk commented Sep 29, 2023

Hi !
I just get an IndexError: pop index out of range when trying to create a Topology like this:

groups = [list(i) for i in zip(*gdf.groupby("datetime"))]
tp.Topology(data=groups[1], object_name=groups[0])

It works fine with two objects in the list (as in this test) but it seems to break for longer list. The issue comes from here:

for ix in range(1, len(geom)):
    geom[0].update(geom.pop(ix))
data = geom[0]

It breaks because pop reduce the length of the list and so the last indices cannot be reached. Simply using geom[ix] instead of geom.pop(ix) should fix it. I'm going to test it.

@mattijn
Copy link
Owner

mattijn commented Sep 30, 2023

Thanks for raising the issue. If you can submit a PR if you have a fix for it, that would be great.
Otherwise a simple example how I can reproduce this error would be fine too!
Thanks again for reaching out!

@aspyk
Copy link
Contributor Author

aspyk commented Oct 3, 2023

Ok thank you for your quick answer ! I've tried the fix locally and it seems to work, so I'm going to try to modify the unit test (ie using simply a list with more than two elements) and then submit a PR :)

@aspyk
Copy link
Contributor Author

aspyk commented Oct 10, 2023

PR done, don't hesitate if something need to be changed :)

@mattijn
Copy link
Owner

mattijn commented Oct 10, 2023

Great @aspyk! You are almost there!
You've created a fork and made the changes.
If it is right there should now appear a green button on the github page of your forked repo of topojson stating, "compare & pull request".
If you click that button and add a title plus description in the new screen and then click "create pull request", it is official.

@aspyk
Copy link
Contributor Author

aspyk commented Oct 10, 2023

Hum I thought I already did that... Is this not working ? #204

@mattijn
Copy link
Owner

mattijn commented Oct 10, 2023

Yes, now I see it. Maybe I hadn't refreshed the page.. thanks for the PR! Will check from there

@aspyk
Copy link
Contributor Author

aspyk commented Oct 10, 2023

I see there is a problem on the CI, do you know what it is ?

@mattijn
Copy link
Owner

mattijn commented Oct 10, 2023

Probably a mistake on my side.. I hope to check somewhere this week

@aspyk
Copy link
Contributor Author

aspyk commented Oct 10, 2023

ok, let me know if I can help on my side !

mattijn added a commit that referenced this issue Oct 11, 2023
Co-authored-by: Mattijn van Hoek <mattijn@gmail.com>
@mattijn
Copy link
Owner

mattijn commented Oct 11, 2023

fixed by #204

@mattijn mattijn closed this as completed Oct 11, 2023
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

No branches or pull requests

2 participants