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.

Sign up for GitHub

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

Jump to bottom

Remove cube iter #3656

Merged
merged 7 commits into from
Feb 24, 2020
Merged

Remove cube iter #3656

merged 7 commits into from
Feb 24, 2020

Conversation

jonseddon
Copy link
Contributor

This change has been discussed with @lbdreyer. A TypeError with a useful message is still raised by Python if a Cube is iterated over but isinstance(cube, collections.Iterable) now behaves as would be expected. This behaviour is documented in https://docs.python.org/3/reference/datamodel.html#special-method-names.

@DPeterK
Copy link
Member

DPeterK commented Feb 13, 2020

@jonseddon this looks good, thanks for putting it together! I wonder whether it would be worth a comment to this in the Iris user guide - probably in §5.2 - cube iteration?

It could be reasonably simple - something like the following, for example:

It is not possible to directly iterate over an Iris cube. That is, you cannot call `cube.next()`. 
Instead you can iterate over cube slices, as this section details.

@jonseddon
Copy link
Contributor Author

@jonseddon this looks good, thanks for putting it together! I wonder whether it would be worth a comment to this in the Iris user guide - probably in §5.2 - cube iteration?

@DPeterK Thanks! Good suggestion. That's added now and the documentation's building successfully.

@rcomer rcomer linked an issue Feb 13, 2020 that may be closed by this pull request
Decide whether cubes are iterable #3186
Closed
pp-mo
pp-mo reviewed Feb 14, 2020
View reviewed changes
docs/iris/src/userguide/subsetting_a_cube.rst Outdated
@@ -103,6 +103,9 @@ same way as loading with constraints:

Cube iteration
^^^^^^^^^^^^^^^
It is not possible to directly iterate over an Iris cube. That is, you cannot call `cube.next()` as
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's very good (modern) Python usage to call a next method directly. Cubes were once iterable by virtue of being indexable #273, but they never did have an actual 'next' function anyway.
So, I think it would be better here to talk about next(cube), or list(cube) or for x in cube.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pp-mo Thanks for the review and suggestion. In 2a4f3d2 I've simplified the language slightly, but I think that this should be easily understandable by typical Iris users.

@DPeterK
Copy link
Member

DPeterK commented Feb 18, 2020

Thanks @jonseddon! I think the update to the userguide sounds good 👍

@pp-mo
Copy link
Member

pp-mo commented Feb 24, 2020

I'm 👍 , think this is good to go.

Any final comment @lbdreyer -- is this what you were expecting ?
( Just because you were namechecked ! )

@lbdreyer
Copy link
Member

Any final comment @lbdreyer -- is this what you were expecting ?

Looks good to me 👍

@pp-mo pp-mo merged commit cbfbbb2 into SciTools:master Feb 24, 2020
@jonseddon jonseddon deleted the remove-cube-iter branch February 24, 2020 15:05
MoseleyS added a commit to MoseleyS/iris that referenced this pull request Mar 6, 2020
@MoseleyS
Merge branch 'master' into nimrod_file_format
5e63577 
* master:
  Remove TestGribMessage (SciTools#3672)
  Removed iris.tests.integration.test_grib_load and related CML files. (SciTools#3670)
  Removed grib-specific test to iris-grib. (SciTools#3671)
  Fixed asv project name to 'scitools-iris'. (SciTools#3660)
  Remove cube iter (SciTools#3656)
  Remove test_grib_save.py (SciTools#3669)
  Remove test_grib2 integration tests (SciTools#3664)
  Remove uri callback test which is moved to iris-grib (SciTools#3665)
  2v4 mergeback picks (SciTools#3668)
  Remove test_grib_save_rules.py which has been moved to iris-grib (SciTools#3666)
  Removed ununused skipIf. (SciTools#3632)
  Remove grib-specific test. (SciTools#3663)
  Remove obsolete test. (SciTools#3662)
  remove redundant tests (SciTools#3650)
  Fixed tests since Numpy 1.18 deprecation of non-int num arguments for linspace. (SciTools#3655)
trexfeathers added a commit that referenced this pull request Mar 18, 2020
@pp-mo @trexfeathers
@bjlittle @lbdreyer @jonseddon
Update ng-vat with recent changes+fixes from master (#3685)
df579a3 
* Fixed tests since Numpy 1.18 deprecation of non-int num arguments for linspace. (#3655)

* remove redundant tests (#3650)

* Remove obsolete test. (#3662)

* Remove grib-specific test. (#3663)

* Removed ununused skipIf. (#3632)

* Remove test_grib_save_rules.py which has been moved to iris-grib (#3666)

* 2v4 mergeback picks (#3668)

* Stop PPDataProxy accessing the file when no data is needed. (#3659)

* Add 2.4 whatsnew into full whatsnew list.

Co-authored-by: Martin Yeo <[email protected]>

* Remove uri callback test which is moved to iris-grib (#3665)

* Remove test_grib2 integration tests (#3664)

* Remove test_grib_save.py (#3669)

* Remove cube iter (#3656)

* Fixed asv project name to 'scitools-iris'. (#3660)

* Removed grib-specific test to iris-grib. (#3671)

* Removed iris.tests.integration.test_grib_load and related CML files. (#3670)

* Remove TestGribMessage (#3672)

* Switched use of datetime.weekday() to datetime.dayofwk. (#3687)

* New image hashes for mpl 3x2 (#3682)

* New image hash for iris.test.test_plot.TestSymbols.test_cloud_cover with matplotlib 3.2.0.

* Further images changes for mpl3x2.

* Yet more updated image results.

* Correct and improve dev-guide section on fixing graphics-tests. (#3683)

* Correct and improve dev-guide section on fixing graphics-tests.

* Review changes + general rethink.

* Reduce duplication between 'graphics-tests' and general 'tests' page.

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

Co-authored-by: Martin Yeo <[email protected]>

Co-authored-by: Martin Yeo <[email protected]>
Co-authored-by: Bill Little <[email protected]>
Co-authored-by: lbdreyer <[email protected]>
Co-authored-by: Jon Seddon <[email protected]>
stephenworsley added a commit to stephenworsley/iris that referenced this pull request Jun 8, 2020
@stephenworsley
Merge branch 'default_units' of https://github.com/SciTools/iris into…
4f267ad 
… default_units_patch

* 'default_units' of https://github.com/SciTools/iris:
  Unify saving behaviour of "unknown" and "no_unit" (SciTools#3711)
  Change default loading unit from "1" to "unknown" (correct branch) (SciTools#3709)
  Change default units to "unknown" for all DimensionalMetadata (SciTools#3713)
  Update docs CubeList.extract method (SciTools#3694)
  Correct and improve dev-guide section on fixing graphics-tests. (SciTools#3683)
  New image hashes for mpl 3x2 (SciTools#3682)
  Switched use of datetime.weekday() to datetime.dayofwk. (SciTools#3687)
  Remove TestGribMessage (SciTools#3672)
  Removed iris.tests.integration.test_grib_load and related CML files. (SciTools#3670)
  Removed grib-specific test to iris-grib. (SciTools#3671)
  Fixed asv project name to 'scitools-iris'. (SciTools#3660)
  Remove cube iter (SciTools#3656)
  Remove test_grib_save.py (SciTools#3669)
  Remove test_grib2 integration tests (SciTools#3664)
  Remove uri callback test which is moved to iris-grib (SciTools#3665)
  2v4 mergeback picks (SciTools#3668)
  Remove test_grib_save_rules.py which has been moved to iris-grib (SciTools#3666)
  Removed ununused skipIf. (SciTools#3632)
  Remove grib-specific test. (SciTools#3663)
  Remove obsolete test. (SciTools#3662)
tkknight pushed a commit to tkknight/iris that referenced this pull request Jun 29, 2020
@jonseddon @tkknight
Remove cube iter (SciTools#3656)
59c085a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pp-mo pp-mo pp-mo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Decide whether cubes are iterable
4 participants
@jonseddon @DPeterK @pp-mo @lbdreyer