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

addcyclic slicer IndexError #555

Closed
fragkoul opened this issue Oct 2, 2022 · 4 comments · Fixed by #559
Closed

addcyclic slicer IndexError #555

fragkoul opened this issue Oct 2, 2022 · 4 comments · Fixed by #559

Comments

@fragkoul
Copy link

fragkoul commented Oct 2, 2022

First of all, kudos to @molinav for keeping basemap alive. I still find it nicer than Cartopy.

Earlier python versions give a FutureWarning and newer python versions an IndexError on the output of the addcyclic function:
version 1.3.4, line 5098: npsel.concatenate((a,a[slicer]),axis=axis)

I suppose this needs to change to:
npsel.concatenate((a,a[tuple(slicer)]),axis=axis) ?

@molinav
Copy link
Member

molinav commented Oct 4, 2022

Hi @fragkoul! Thanks for your support and for reporting the issue.

I may have some time around Friday to check it properly, but you are very likely right, and your solution is the correct one. I remember seeing around this numpy warning for very long time, so it was a matter of time that they would stop supporting this old syntax.

If you want to try out, feel also free to provide a pull request with this change and I will accept it. If you have a simple minimal example, it would also be very nice, since I would like to start adding some basic tests to the repository based on the bugs found by the users. If you do not have time, do not worry, I will come back to it on Friday.

@molinav
Copy link
Member

molinav commented Oct 19, 2022

@fragkoul Sorry for the late feedback, I was busier than expected. I checked three example scripts from the examples folder that make use of addcyclic and I got the exact same error as you. Applying your proposed fix solves the problem, so I have just committed it to the repo.

When the pipelines pass, I will prepare a hotfix release 1.3.5 with the bugfix. I would also like to include in this hotfix the issue #522 because it is a very problematic one, so it might take me some days until I tag it (but hopefully soon).

@molinav
Copy link
Member

molinav commented Oct 19, 2022

The patch is merged into the hotfix-1.3.5 branch. Thanks for reporting the issue and the solution!

@molinav molinav closed this as completed Oct 19, 2022
@molinav
Copy link
Member

molinav commented Oct 25, 2022

The bugfix is already available in the latest basemap release 1.3.5 (on PyPI and conda-forge).

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 a pull request may close this issue.

2 participants