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

dask: cf.Field.regrids and cf.Field.regridc #438

Merged
merged 87 commits into from
Jan 30, 2023

Conversation

davidhassell
Copy link
Collaborator

I'm finished. In many ways.

Hopefully self explanatory ... might be easiest to start at cf.Field.regrid[sc] and work backwards from there.

I'll draw a bit of UML-like to outline the structure. Watch this space.

You need to run python create_test_files.py before testing for the first time.

Thanks!

davidhassell and others added 23 commits January 30, 2023 18:01
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
@davidhassell
Copy link
Collaborator Author

Addressing the general minor comments:

  1. test_Field references one of the old regrid_fileN.ns files (N=1)

We'll open a new PR for this.

  1. I'm getting a docstring substitution error raised via the test.

Sorted: 2ca3e2c

  1. When I run test_regrid, I get repeated (hundreds of) deprecation warnings coming through from ESMF.

As discussed, Ankit and I don't :) We'll put it down to rogue versions and ignore.

  1. Some of the old RegridOperator methods that have been deleted here are still being listed in the documentation

Sorted: ee57d6e

  1. Flake8 is reporting some errors.

Annoying, but we'll ignore for now. It's possible that when merged into lama-to-dask they'll get fixed, as this quite an old PR. Maybe!

Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

Feedback all addressed very well. Lovely, please merge.

@davidhassell davidhassell merged commit 17ad528 into NCAS-CMS:lama-to-dask Jan 30, 2023
@davidhassell
Copy link
Collaborator Author

A very large thank you to Sadie for tackling the review of this rather large PR. Nice job.

@sadielbartholomew
Copy link
Member

(I've since realised my main initial review comment got mangled, but understood and discussed offline anyway.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dask Relating to the use of Dask top priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants