Skip to content

reimplementation of pole_kind option#415

Open
raphaeldussin wants to merge 2 commits into
masterfrom
testing_pole_kinds2
Open

reimplementation of pole_kind option#415
raphaeldussin wants to merge 2 commits into
masterfrom
testing_pole_kinds2

Conversation

@raphaeldussin

Copy link
Copy Markdown
Contributor

this PR replaces #236 and #223

testing uses sample data from figshare.

This was referenced Feb 9, 2025

@aulemahal aulemahal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A few suggestions.

An example in one of the notebooks could be interesting ?

file_to_rem.unlink()
file_to_rem = pathlib.Path('xesmf_testing_OM4.tar.gz')
file_to_rem.unlink()
return None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return None

xe.SpatialAverager(ds_in, [poly])


def test_regrid_polekind():

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def test_regrid_polekind():
def test_regrid_polekind(tmp_path):

I suggest writing the test files in a temporary path (see https://docs.pytest.org/en/7.1.x/how-to/tmp_path.html).

As a bonus, you don't even need to clean them up at the end.

Comment thread xesmf/frontend.py
Comment on lines +160 to +161
if 'pole_kind' in ds:
pole_kind = np.asarray(ds['pole_kind'])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The possibility of passing pole_kind in the dataset should maybe be added somewhere in the documentation ? The docstring of Regridder maybe ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants