Skip to content

Conversation

@ESadek-MO
Copy link
Contributor

🚀 Pull Request

Addresses step one of #5399.

Takes code from #4572 and solves conflicts with latest changes in Iris.

@ESadek-MO ESadek-MO added the Type: Feature Branch Highlight this for a feature branch label Nov 6, 2023
@codecov
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (e0850eb) 89.44% compared to head (7117a23) 89.43%.

❗ Current head 7117a23 differs from pull request most recent head 818a6f2. Consider uploading reports for the commit 818a6f2 to get more accurate results

Additional details and impacted files
@@                    Coverage Diff                    @@
##           FEATURE_chunk_control    #5565      +/-   ##
=========================================================
- Coverage                  89.44%   89.43%   -0.01%     
=========================================================
  Files                         89       89              
  Lines                      22583    22650      +67     
  Branches                    5386     5406      +20     
=========================================================
+ Hits                       20199    20257      +58     
- Misses                      1638     1643       +5     
- Partials                     746      750       +4     
Files Coverage Δ
lib/iris/_lazy_data.py 98.41% <95.74%> (-1.59%) ⬇️
lib/iris/fileformats/netcdf/loader.py 81.19% <85.10%> (+0.46%) ⬆️

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

@ESadek-MO
Copy link
Contributor Author

ESadek-MO commented Nov 10, 2023

Chose to use collections instead of typing where possible, as outlined here: https://docs.python.org/3/library/typing.html#deprecated-aliases

I could find no reference to Union in collections, so kept that as was.

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.

This code holds together very nicely 👍

I know my notes relate to documentation and testing, which will be fully addressed later, but I'd rather not introduce problems when they have already been spotted.

@trexfeathers trexfeathers merged commit f73f479 into SciTools:FEATURE_chunk_control Nov 13, 2023
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 <[email protected]>

---------

Co-authored-by: Patrick Peglar <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Martin Yeo <[email protected]>

* 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 <[email protected]>
Co-authored-by: Patrick Peglar <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@ESadek-MO ESadek-MO deleted the chunk_control_unstale 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

Type: Feature Branch Highlight this for a feature branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants