Skip to content

Fix handling of PosixPath in cartopy.io fh_getter function #2500

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

Merged
merged 1 commit into from
Apr 13, 2025

Conversation

jolivetr
Copy link
Contributor

Rationale

fh_getter fails when fh is a PosixPath because it attempts at checking filename, which has not been initialized.

Fix

Initialize filename to None

Implications

Minimal

fh_getter fails when fh is a PosixPath because it attempts at checking filename, which has not been initialized.

Fix: Initialize filename to None
@jolivetr
Copy link
Contributor Author

A very tiny fix in fh_getter for cartopy.io.
A variable was tested and not initialized.
Fix: Intitialize the variable

@dopplershift
Copy link
Contributor

I like the fix, I'm wondering if we can add a test. What were you doing to trigger this error in the first place?

@QuLogic
Copy link
Member

QuLogic commented Feb 20, 2025

Please update the commit message/PR title to be clearer about the change.

@jolivetr jolivetr changed the title Update __init__.py Fix handling of PosixPath in cartopy.io fh_getter function Feb 20, 2025
@jolivetr
Copy link
Contributor Author

Done. Let me know if this is ok

@jolivetr
Copy link
Contributor Author

I like the fix, I'm wondering if we can add a test. What were you doing to trigger this error in the first place?

I am not sure. I am using cartopy to plot SRTM data. I think this error shows up with recent versions of Python (12 and 13 for sure). It was not failing with 3.8.

Anyway, the variable filename was not initialized in the first place, so that should fix it.

@greglucas greglucas merged commit d686f36 into SciTools:main Apr 13, 2025
23 checks passed
@QuLogic QuLogic added this to the 0.25 milestone Aug 10, 2025
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 this pull request may close these issues.

4 participants