-
Notifications
You must be signed in to change notification settings - Fork 297
Use numpy.linspace to generate grids built with reference point and spacings #4113
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
Conversation
6bfc725
to
dada6a4
Compare
Hi @gcaria, thanks for this. We are currently having multiple problems with our Cirrus CI testing, so the test output will be showing a whole raft of failures that are not linked to your change. To get everything working again, we need to merge #4108, #4104, #4112 and probably something to address #3993. So please bear with us while we sort all that out, then you can rebase and someone can have a proper look at this. Again, thanks for your contribution! |
Hi @gcaria, we have now got the tests all passing on |
dada6a4
to
29e7a8c
Compare
Thanks for the feedback @rcomer ! It seems that the test failures are then caused by this commit, although it's hard to believe such a simple edit can break anything. Maybe this has to do with some expected reference data that doesn't match the actual one anymore? I admit I have some difficulties running the test suite on my own, since I get a lot of |
Thanks for the update @gcaria. Yes, it looks like this is causing very small changes all over the place, and in some cases it does look for the worse. E.g. here the latitude points change from round multiples of 0.5 to ?.?999. Some of the test input files are in pp-format, and the pp file headers only specify latitude/longitude by their "zeroth" value, step and number of rows/columns. So that probably explains why so many unrelated tests are affected. I'm not sure what the right thing to do here is. It seems like a computer science question to me, and I am very much not a computer scientist! Perhaps @alastair-gemmell might have some ideas, since he opened the original issue. |
I tried a close and reopen to nudge the CI, but I think it's mostly just looking at quite old tests. A rebase would fix that. When I run the tests locally with this change, I get the CML match failures and a couple of other errors from In terms of whether the CML failures are a bad thing, I think they'd all/mostly pass an Before merging this change, I think that we should update the lines just below it that use |
We have discussed this @SciTools/peloton and have decided to close this; please feel free to reopen! Thanks for your work on this! |
Close #3559
A very simple fix taken straight from the issue's only comment.