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

Chunk Control Tests #5583

Merged
merged 24 commits into from
Nov 20, 2023
Merged

Conversation

ESadek-MO
Copy link
Contributor

@ESadek-MO ESadek-MO commented Nov 15, 2023

🚀 Pull Request

Closes #5399.

Description

Adds tests for changes made on branch FEATURE_chunk_control.

  • move tests to netcdf file, and rename to remove leading netcdf_
  • Convert existing tests to PyTest
  • Add tests for -1 functionality
  • Add test for from_file context manager
  • Add test for as_dask context manager
  • Add test for as_lazy_data to ensure dask bypassing works

Tests are in some cases close enough to add parameterisation, but I'm not convinced it would be readable enough, and would need to pass in parameters and results.

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (FEATURE_chunk_control@a979fdc). Click here to learn what that means.

❗ Current head 6727f7b differs from pull request most recent head 929b03b. Consider uploading reports for the commit 929b03b to get more accurate results

Additional details and impacted files
@@                   Coverage Diff                    @@
##             FEATURE_chunk_control    #5583   +/-   ##
========================================================
  Coverage                         ?   89.44%           
========================================================
  Files                            ?       89           
  Lines                            ?    22689           
  Branches                         ?     5414           
========================================================
  Hits                             ?    20294           
  Misses                           ?     1644           
  Partials                         ?      751           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Here's some stuff.

Everything that is written is written well. I still need to think about whether anything is missing.

lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/integration/netcdf/test__chunk_control.py Outdated Show resolved Hide resolved
lib/iris/tests/unit/lazy_data/test_as_lazy_data.py Outdated Show resolved Hide resolved
lib/iris/tests/unit/lazy_data/test_as_lazy_data.py Outdated Show resolved Hide resolved
Copy link
Contributor

@trexfeathers trexfeathers Nov 16, 2023

Choose a reason for hiding this comment

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

Testing that @pp-mo felt was worth adding (#4572 (comment)):

Also, by pinning only certain dimensions, I can get the automatic operation to optimise the sizes in the other dims.
E.G. if I had automatic chunks of (1, 1, 100, 1000, 2000) ; if I then pin the 3rd dim to 3 I will then get (1, 33, 3, 1000, 2000) instead, which is still "optimally" sized.
(TBD: this code needs properly testing !!)

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.

Need something in test__get_cf_var_data.py. Not a full replication of what we have written elsewhere, but just something to demonstrate that _get_cf_var_data() responds to CHUNK_CONTROL.

@trexfeathers trexfeathers merged commit 34571eb into SciTools:FEATURE_chunk_control Nov 20, 2023
13 of 16 checks passed
ESadek-MO added a commit that referenced this pull request Nov 23, 2023
* Merge chunk control code into latest iris (#5565)

* Dask chunking control for netcdf loading.

* renamed loader

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix indentation error, perhaps also docstring error

* fixed result error in loader, and set tests to treat as big files

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* trial and error, solve non iterable tuple 1.0

* trial and error, solve non iterable tuple 2.0 (used if var is none: instead of if var: )

* commented out docstring

* fixed mock 'no name' failure

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fixed precommit issues

* corrected docstrings as per review comments

* Removed unnecessary line

Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com>

---------

Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com>

* Chunk control modes (#5575)

* added modes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* added as_dask mode

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* cleaned up enum and as_dask, as per review comments

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* corrected  to  in final place

* unindented lazy_param assignment one indent

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Corrected required type of dimension_chunksizes. (#5581)

* Chunk Control Tests (#5583)

* converted tests to pytest, added neg_one, and incomplete from_file and as_dask tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* added from_file test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* added mocking tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* trial and error with mocks and patches, may or may not work

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* converted Mock to patch in as_dask test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* review comment changes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* pre commit fixes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* review comments, and added test in test__get_cf_var_data()

* added in another test

* added tests and fixed review comments

* added AuxCoord test

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Chunk control minor fixes (#5593)

* Disallow chunks=None in optimum_chunksize.

* Clearer docstrings.

* Corrected docstring.

* Chunk Control documentation (#5597)

* init PR, skeleton TP

* whoops, missed the TP.

* fixed doctests in rst file

* correct triple chevron to elipses

* updated set doctest to better show functionality

* removed in-progress doctest code

* Review comments, part 1

* Review comments, part 2

* changed numpy docs dict

* wait, this way is better

* fixed linkcheck failures (maybe)

* fixed :meth:

* fixed a couple doc bits

* hopefully fixed doctests

* newest review comments

* fixed rendering, and wording in docstring

* fixed docstring numpyness

* What's New Entry (#5601)

* written whatsnew entry

* added ref

* moved label to before title

---------

Co-authored-by: Elias <110238618+ESadek-MO@users.noreply.github.com>
Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@ESadek-MO ESadek-MO deleted the cc_tests branch November 23, 2023 15:43
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.

2 participants