Skip to content

Conversation

@Badboy-16
Copy link
Contributor

🚀 Pull Request

Description

I had a go to resolve issue #4066 .
A method copy() was created in the CubeList class to return a CubeList object instead of a list.


Consult Iris pull request check list

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.

Hi @Badboy-16 , thanks for raising this - we really appreciate our users becoming collaborators 🙂. And sorry for our absence on this - we've been away on some intense project work!

Your solution looks ideal 🍻, but as mentioned in the pull request checklist we will also need an entry in What's New.

I also think it would be sensible to add a test confirming this is working as expected. If you'd like any help with this just shout.

Unfortunately in April we had some continuous integration issues, which we have since fixed - to bring in these fixes you will need to rebase your PR.

Looking forward to hearing from you!

@trexfeathers trexfeathers self-assigned this May 19, 2021
@SciTools-assistant SciTools-assistant added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label May 19, 2021
@trexfeathers trexfeathers added Peloton 🚴‍♂️ Target a breakaway issue to be caught and closed by the peloton and removed Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form labels May 19, 2021
@SciTools-assistant SciTools-assistant added Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form labels May 19, 2021
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.

Thanks for your persistence - almost there!

In addition to the 2 comments, please could you sign the Contributor Licence Agreement - required for all Iris' pull requests.

Many Thanks.

self.assertIs(cube.__ne__(Terry()), NotImplemented)


class Test_CubeList_copy(tests.IrisTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

This belongs in tests/unit/cube/test_CubeList.py

I fully acknowledge Iris' testing structure is unclear!

printing these objects skips metadata elements that are set to None or an
empty string or dictionary. (:pull:`4040`)

#. `@Badboy-16`_ implemented a ``CubeList.copy()`` method to return a
Copy link
Contributor

Choose a reason for hiding this comment

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

This link also needs creating toward the bottom of the file - currently L172-174.

That will fix the test failures.

@SciTools-assistant SciTools-assistant removed the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label May 27, 2021
@Badboy-16
Copy link
Contributor Author

Hi @trexfeathers
Thanks for your comments! I have signed the agreement and pushed some commits regarding the requested changes. :)

@trexfeathers trexfeathers linked an issue May 27, 2021 that may be closed by this pull request
@trexfeathers trexfeathers merged commit 6cab227 into SciTools:master May 27, 2021
tkknight added a commit to tkknight/iris that referenced this pull request Jun 4, 2021
* master:
  refactor setup.py to setup.cfg (SciTools#4168)
  update docs pypi release (SciTools#4173)
  Update CI environment lockfiles (SciTools#4137)
  update CONTRIBUTING.md (SciTools#4165)
  RTD support link update (SciTools#4166)
  drop py36 support (SciTools#4163)
  github issues contact link for discussions (SciTools#4164)
  Bump black version (SciTools#4162)
  Stop CI from clobbering commits on lockfile updates (SciTools#4157)
  [pre-commit.ci] pre-commit autoupdate (SciTools#4161)
  Add a method to return a CubeList from CubeList.copy() (SciTools#4094)
  Update black et al (SciTools#4155)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Peloton 🚴‍♂️ Target a breakaway issue to be caught and closed by the peloton

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CubeList.copy() should return a CubeList

3 participants