8000 Test the `popitem` method of `MutableMapping`s too by jakirkham · Pull Request #378 · zarr-developers/zarr-python · GitHub
[go: up one dir, main page]

Skip to content

Test the popitem method of MutableMappings too #378

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

Merged
merged 1 commit into from
Jan 16, 2019
Merged

Test the popitem method of MutableMappings too #378

merged 1 commit into from
Jan 16, 2019

Conversation

jakirkham
Copy link
Member
@jakirkham jakirkham commented Jan 4, 2019

Adds a test for the popitem method of MutableMapping's as well. Typically this is not used, but it does get used in the default clear method. Given this method doesn't guarantee an order, use a store with a single-key value pair to simplify the test logic. Also test the case of an empty store to make sure it errors out properly. Override the popitem method appropriate for stores that do not handle removal properly.

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • Docs build locally (e.g., run tox -e docs)
  • AppVeyor and Travis CI passes
  • Test coverage is 100% (Coveralls passes)

@jakirkham
Copy link
Member Author

Thoughts @zarr-developers/core-devs?

@jhamman
Copy link
Member
jhamman commented Jan 14, 2019

LGTM.

Adds a test for the `popitem` method of `MutableMapping`'s as well.
Typically this is not used, but it does get used in the default `clear`
method. Given this method doesn't guarantee an order, use a store with a
single-key value pair to simplify the test logic. Also test the case of
an empty store to make sure it errors out properly. Override the
`popitem` method appropriate for stores that do not handle removal
properly.
@jakirkham jakirkham merged commit 585f0f5 into zarr-developers:master Jan 16, 2019
@jakirkham jakirkham deleted the tst_popitem branch January 16, 2019 01:09
@jakirkham
Copy link
Member Author

Thanks :)

Have gone ahead and merged it. If anyone spots an issue, please let me know and we can follow-up on it.

@jakirkham jakirkham added this to the v2.3 milestone Feb 17, 2019
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
0