Bug fixes to arcgisimage

matplotlib / basemap

Plot on map projections (with coastlines and political boundaries) using matplotlib

MIT License

772 stars 390 forks source link

Bug fixes to arcgisimage #598

Closed nitram96 closed 5 months ago

nitram96 commented 5 months ago

fixed bugs with using cachedir in arcgisimage. cachedir is no longer ignored if verbose is false if a cached image is found, now correctly shows it using self.imshow() changed to use os.path.join to join cachedir and filename, so a backslash is no longer required at the end of cachedir

molinav commented 5 months ago

Thanks, @nitram96! It is clear that your changes are correct, let's wait for the workflows to finish and I merge it.

Hopefully I will find some time to increase the coverage of the library. With more tests I would not have overlooked this bug.

molinav commented 5 months ago

@nitram96 Sorry for the delay, everything looks fine, but I would like to add a couple of unit tests before merging. I hope to find some time within the next 2 days, and then most likely I will tag as basemap release 1.4.1 after the merge, because having such method broken in the latest stable is not nice.

molinav commented 5 months ago

@nitram96 I took the opportunity that you opened the PR to fix a bit more the Basemap.arcgisimage method:

The method code is now compliant with the most basic flake8 and pylint requirements.
The docstring has been reformated so that it will get rendered correctly by Sphinx with the current furo theme.
The cache directory should be created on the fly if saving a cache image for the first time and cachedir is a string corresponding to a non-existing directory. This code block was located at the wrong place, so I moved it accordingly.
I added a couple of tests for Basemap.arcgisimage using the cachedir argument, one with an existing directory and another with a still non-existing directory.

molinav commented 5 months ago

All seems in order, so I am merging. Thanks again, @nitram96!