Skip to content

Remove iris-grib dependency from Iris tests. #3742

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Jun 25, 2020
Merged

Remove iris-grib dependency from Iris tests. #3742

merged 12 commits into from
Jun 25, 2020

Conversation

@pp-mo
Copy link
Member

@pp-mo pp-mo commented Jun 17, 2020 •
edited
Loading

Removes iris-grib from the iris tests.
We no longer need iris-grib for any Iris testing - it has now all moved to iris-grib.

This includes the removal of the 'fileformat interoperability' tests tests/integration/format_interop
Really, these were all testing grib saving, and have been moved to iris-grib : see SciTools/iris-grib#213

Unfortunately GRIB support is still used by one of the gallery examples, so I wasn't able to completely remove it from .travis.yml.
See : #3745 (to complete the job)

@pp-mo pp-mo force-pushed the remove_ff_interop_tests branch from 82c6972 to b56eb9f Compare June 17, 2020 16:15
@pp-mo pp-mo force-pushed the remove_ff_interop_tests branch from 797462a to 362af05 Compare June 17, 2020 18:39
@pp-mo
Copy link
Member Author

pp-mo commented Jun 18, 2020 •
edited
Loading

one of the gallery examples

That is, docs/iris/example_code/General/polar_stereo.py.
Frustratingly, the GRIB data this uses has a stereographic coordinate system with a 'true scale latitude', and we don't support true-scale-lat for cf load/save.
So I can't replace with a netcdf version of the data, which would have been a nice solution.

@pp-mo pp-mo changed the title Remove file-format interoperability tests (gone to iris-grib). Remove iris-grib dependency from Iris tests. Jun 18, 2020
@pp-mo pp-mo mentioned this pull request Jun 18, 2020
Closed
@pp-mo pp-mo added this to the v3.0.0 milestone Jun 18, 2020
trexfeathers
trexfeathers requested changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@trexfeathers trexfeathers left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pp-mo, thanks for the PR, I agree with all the changes you've made.

But based on a search of the remaining source code I think there might still be more to do, or I might be misunderstanding the code; please could you explain/address each of the following references to GRIB:

@pp-mo
Copy link
Member Author

pp-mo commented Jun 24, 2020

Hi @trexfeathers thanks for help.
Regarding your points -- I think some of that removing or fixing refs to grib/iris-grib goes a bit beyond what I was trying to achieve here, but they are all good spots so I will try + fix them.
Watch this space ...

In the meantime, if we are also close to getting #3746 in, then we can follow this up by removing all use of iris-grib.
I'll either do that here, or afterwards+separately depending how the timings fall out.

trexfeathers
trexfeathers requested changes Jun 24, 2020
View reviewed changes
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a single change that needs rolling back

@trexfeathers trexfeathers mentioned this pull request Jun 24, 2020
Merged
@pp-mo
Copy link
Member Author

pp-mo commented Jun 24, 2020

Sorry for slow response @trexfeathers
Distracted elsewhere. Hopefully that resolves it

trexfeathers
trexfeathers approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @pp-mo!

@trexfeathers trexfeathers merged commit a8f90ef into SciTools:master Jun 25, 2020
tkknight added a commit to tkknight/iris that referenced this pull request Jun 25, 2020
@tkknight
Merge branch 'master' into docs_overhaul
acb5de9 
* master:
  Remove iris-grib dependency from Iris tests. (SciTools#3742)
tkknight added a commit to tkknight/iris that referenced this pull request Jun 29, 2020
@tkknight
Merge branch 'docs_overhaul' of github.com:tkknight/iris into docs_ov…
63a06cf 
…erhaul

* 'docs_overhaul' of github.com:tkknight/iris:
  some changes
  Clean build (almost)
  tidy README
  tidy
  PI-3745: Stereographic plotting example using netcdf instead of GRIB data (SciTools#3746)
  svg to png
  reinstate grib examples
  minor tweak
  Remove iris-grib dependency from Iris tests. (SciTools#3742)
  added download option for pdf
  travis test for linkcheck
  travis test
  new travis job to run linkcheck
  fixed links
  tidy rtd build
  rtd test 16
  rtd test 15
  rtd test 14
  rtd test 13
  rtd test 12
tkknight added a commit to tkknight/iris that referenced this pull request Jun 29, 2020
@tkknight
Merge branch 'docs_overhaul' of github.com:tkknight/iris into docs_ov…
78267b5 
…erhaul

* 'docs_overhaul' of github.com:tkknight/iris: (122 commits)
  squash it
  some changes
  Clean build (almost)
  tidy README
  tidy
  PI-3745: Stereographic plotting example using netcdf instead of GRIB data (SciTools#3746)
  svg to png
  reinstate grib examples
  minor tweak
  Remove iris-grib dependency from Iris tests. (SciTools#3742)
  added download option for pdf
  travis test for linkcheck
  travis test
  new travis job to run linkcheck
  fixed links
  tidy rtd build
  rtd test 16
  rtd test 15
  rtd test 14
  rtd test 13
  ...
@pp-mo pp-mo deleted the remove_ff_interop_tests branch February 17, 2021 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trexfeathers trexfeathers trexfeathers approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@pp-mo @trexfeathers