search

geopandas / pyogrio

Vectorized vector I/O using OGR
https://pyogrio.readthedocs.io
MIT License
258 stars 22 forks source link

BUG: undefined variable in timezone logic #370

Closed m-richards closed 4 months ago

m-richards commented 4 months ago

I don't know how I missed this in #253, but there's a logical error if an exception is thrown in this block.

I'm not quite sure how to test this - I only noticed the error in my geopandas dev environment where I had pyogrio 0.7.2 with pandas dev - so I was getting the exception fixedby #348 (not part of 0.7.2), not a future error due to mixed time zones - which is what this try except was originally for.

m-richards commented 4 months ago

Thanks for catching this!

Going to merge now so as to avoid this remaining undefined. However, does this also indicate that instead of catching all exceptions (via except Exception) that there should be different handling for the future error of mixed time zones?

I can have another look but I didn't think this had been done yet - the error I saw was for expiring errors="ignore" - which now throws an assertionerror, I don't imagine that convention would be copied

martinfleis commented 3 months ago

This is now showing up in https://github.com/geopandas/geopandas/pull/3223. Might be worth trying to get a version with this out sooner than later to avoid issues when testing against geopandas main (and to get green CI in geopandas).