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

Make objects actually implement interfaces and update interface definitions #159

Merged
merged 3 commits into from
Apr 7, 2021

Conversation

jamadden
Copy link
Member

@jamadden jamadden commented Mar 31, 2021

  • Make the BTree objects (BTree, TreeSet, Set, Bucket) of each module actually provide the interfaces defined in BTrees.Interfaces. Previously, they provided no interfaces.

  • Update the definitions of ISized and IReadSequence to simply be zope.interface.common.collections.ISized and zope.interface.common.sequence.IMinimalSequence respectively.

  • Remove the __nonzero__ interface method from ICollection. No objects actually implemented such a method; instead, the boolean value is typically taken from __len__.

Fixes #138

(Currently based on #156 to avoid a conflict; only the last commit is relevant here.)

@jamadden jamadden requested a review from icemac April 5, 2021 10:32
Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

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

I like the idea, but there are some open questions.

including, index2.
"""
# Backwards compatibility alias. To be removed in 5.0
IReadSequence = IMinimalSequence
Copy link
Member

Choose a reason for hiding this comment

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

Should the scheduled removal use a deprecation warning to inform users to update their code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Great question!

In theory, yes, it probably should. That would require adding a dependency on zope.deprecation, which this package currently does not have. This package has the extremely minimal requirements of just persistent and zope.interface, and it seems desirable to me to keep the requirements minimal as this is such a low-level package.

So, I'm hoping we can get away with just docs-deprecation. From a practical/pragmatic standpoint, since nothing actually implemented any of these interfaces, there would be little incentive to use them. Searching GitHub finds essentially no references to "BTrees.Interfaces" or "IReadSequence" outside of copies of this package (there's a handful of references no the the BTreesConflictError defined here, but that seems to be it).

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the research on the usage of these interfaces. I am okay with deprecating only in the documentation.

src/BTrees/Interfaces.py Outdated Show resolved Hide resolved
src/BTrees/tests/common.py Outdated Show resolved Hide resolved
- Make the BTree objects (BTree, TreeSet, Set, Bucket)
  of each module actually provide the interfaces defined in
  BTrees.Interfaces. Previously, they provided no interfaces.

- Update the definitions of ISized and IReadSequence to simply
  be zope.interface.common.collections.ISized and
  zope.interface.common.sequence.IMinimalSequence respectively.

- Remove the __nonzero__ interface method from ICollection. No
  objects actually implemented such a method; instead, the boolean value
  is typically taken from __len__.

Fixes #138
Fixes a bunch of warnings importing this package with ZOPE_INTERFACE_LOG_CHANGED_IRO set.
Strengthen the documentation added about the interface changes, and fix the links to IMinimalSequence.
With the extent versions of Sphink and repoze.sphinx.autointerface, you can only link as a module, not as the class.
@jamadden
Copy link
Member Author

jamadden commented Apr 7, 2021

Thank you!

@jamadden jamadden merged commit 49776df into master Apr 7, 2021
@jamadden jamadden deleted the issue138 branch April 7, 2021 10:14
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.

Interface definitions outdated
2 participants