-
Notifications
You must be signed in to change notification settings - Fork 287
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
Ugrid cf load #3684
Ugrid cf load #3684
Conversation
Thanks for some eyes on this @lbdreyer @alastair-gemmell I'm wondering if it is worth getting the test-data up somewhere online? An alternative might be to have more focussed unit-tests, which could work from a synthetic file (somehow?). But I think that is probably overkill + wasted time at this stage, No ? |
My preference would be towards using real data (not synthetic data) so we just need to get approval for a test file we can make public. |
Well, here's a thing. How do you feel about that, @lbdreyer @alastair-gemmell ? |
I suppose it depends on how different the older format is to the newer formats, balanced against the time and effort required to get a more up-to-date test file to the test repo? |
Well it is a valid UGRID format, and I think that maybe good enough for now. In the meantime, there are a bunch of consequential errors in existing tests. |
I fixed the few tests that create a CFReader directly, since its API has changed. The remaining problems were caused by existing regridding tests that use the test file SciTools/iris-test-data#56 introduces a hacked version of that file, |
Hang on a tick please. |
Done. |
Small changes. |
Co-Authored-By: lbdreyer <lbdreyer@users.noreply.github.com>
Yet another small tweak. |
LGTM! 🎉 |
DO NOT MERGEShould re-target this at branch "ng-vat"Did try that, but at present 'ng-vat' is stale, wanting a mergeback from master. See : #3685Do not merge until that is resolved : this can then be rebased onto + re-targetted at ng-vat.#3685 now merged, this PR should be re-pointed at ng-vat.