-
Notifications
You must be signed in to change notification settings - Fork 43
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
alter calculation of offset #287
Conversation
I've updated a couple of tests to mock the |
Hi David, thanks for this. The cryptic message about a CLA (Contributor Licence Agreement) more more helpfully direct you here, which has information and a (quick) form that contributors to SciTools need to fill out: https://docs.google.com/forms/d/e/1FAIpQLSfd0tdE-DcJOXh8ej_7T93IizwJFYBFyRWYQOi2A8QRaKwykA/viewform |
Now signed the CLA |
Hi @david-bentley, sometimes the cla-assistant doesn't pick up when it has been signed. I am going to close and reopen this PR to trigger it to run again |
@david-bentley are you free to rebase / merge in the latest from |
ac243eb
to
69d9936
Compare
@david-bentley if you bring in #339 I think this should pass the CI |
db6f2e9
to
fa65b98
Compare
Just for reference, here are the results of the code in #286 on Windows with these changes: 669134.5234870911 669134.5234870911
offset 0
669579.7834396362 669579.7834396362
offset 3713
669659.5537185669 669659.5537185669
offset 7426
669386.830329895 669386.830329895
offset 11139
667856.9562911987 667856.9562911987
offset 14852
666605.8734893799 666605.8734893799
offset 18565
666640.6002998352 666640.6002998352
offset 22278
666665.0799751282 666665.0799751282
offset 25991
666563.8387680054 666563.8387680054
offset 29704
666692.4885749817 666692.4885749817
offset 33417
666673.7871170044 666673.7871170044
offset 37130
666599.6193885803 666599.6193885803
offset 40843
... Which is what we expect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@david-bentley this is great!
Thanks for your diligence, but even more thanks for your patience.
These changes allow the correct calculation of the message offset on both Windows and Linux.
There are three additional test errors introduced as part of these changes (see below - only tested on Linux so far). These are all related to
grib_message
being a string rather than an integer - I'm not particularly familiar with GRIB, so I'm not sure if this is a peculiarity of the tests or that one would expect to encounter a message as a string when loading a file.