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

Consider adding support for __or__/__ror__ to IntefaceClass #280

Closed
Daverball opened this issue Dec 5, 2023 · 8 comments · Fixed by #285
Closed

Consider adding support for __or__/__ror__ to IntefaceClass #280

Daverball opened this issue Dec 5, 2023 · 8 comments · Fixed by #285

Comments

@Daverball
Copy link

Daverball commented Dec 5, 2023

FEATURE REQUEST

Currently interfaces created with zope.interface do not support the Python3.10 shorthand notation for union types, it makes sense that this is not supported by default, since subclassing Interface creates instances of InterfaceClass which is not a subclass of type.

What I did:

class IFoo(Interface):
    pass

x: IFoo | None = None

What I expect to happen:

IFoo | None results in either typing.Union[IFoo, None] (or types.UnionType(IFoo, None) if it is implemented in a C extension)

What actually happened:

TypeError: unsupported operand type(s) for |: 'InterfaceClass' and 'NoneType'

What version of Python and Zope/Addons I am using:

Python 3.10
zope.interface 6.1

The naive fix for this should be straightforward, I don't think we need to worry about emitting type errors for invalid union members. The standard library is in charge of Union and making it work correctly, so we don't need to do anything special.

from typing import Union

...

class InterfaceClass(_InterfaceClassBase):

    ...

    def __or__(self, other):
        return Union[self, other]

    def __ror__(self, other):
        return Union[other, self]

    ...

This work by David Salvisberg is marked with CC0 1.0 Universal.

@Daverball
Copy link
Author

Looks like there already is a PR for this, I only searched through the issues. #267

@icemac Can we maybe encourage getting this merged by allowing the contributor to use the simplified process? It's a very minimal change after all (I'd even consider it trivial since it has little to no impact on existing zope.interface code), where the zope licensing restrictions seem archaic at best, so I'm not surprised there hasn't been any activity on the PR after that point in time.

@icemac
Copy link
Member

icemac commented Dec 15, 2023

@Daverball I am sorry, the simplified process no longer exists, because it should not have existed in the first place. Maybe sometime a contributor will retry to solve this issue. I personally currently do not have enough steam.

@Daverball
Copy link
Author

That's a shame. I hope the Zope foundation will eventually find a more pragmatic approach to small changes, I am sure you would be getting a lot more contributions for the small things that need to be done to keep up with the various PEPs and improvements in the Python ecosystem as a whole and can free up some personal resources for the larger tasks that need to be done.

The way I see it, as it currently stands, the Zope ecosystem will get further and further behind modern Python features, which will eventually make the cost for migrating to something else seem trivial, compared to the time you have to spend working around limitations in the various Zope packages.

Having to sign an agreement to contribute to an open source package just generally feels gross and will turn many people, myself included, off creating pull requests, even if they had the time and know-how to do it, this will be especially true for contributions from people, whose first language is not English and would not feel confident knowing what they're actually agreeing to when signing the agreement. I probably would have already opened a handful of additional pull requests by now for some small improvements, but now that I know that I have to sign something, I don't really feel like putting in the time.

@icemac
Copy link
Member

icemac commented Dec 15, 2023

I am going to discuss this problem in the steering circle.

@icemac
Copy link
Member

icemac commented Feb 14, 2024

@Daverball As the result of the discussion in the steering circle I updated https://github.com/zopefoundation/.github/blob/master/CONTRIBUTING.md so there is now a way without signing the contributor agreement.

@Daverball
Copy link
Author

@icemac I've updated the issue, I think this should be enough, I don't think we need to worry about unit tests like the other PR, since Union is part of the standard library and thoroughly unit tested there. We can just add the feature based on the code snippet in the issue, i.e. by adding five lines of code, that I've placed under CC0.

@zopefoundation zopefoundation locked as resolved and limited conversation to collaborators Feb 15, 2024
@icemac
Copy link
Member

icemac commented Feb 15, 2024

@Daverball Implemented in #285

@icemac
Copy link
Member

icemac commented Feb 16, 2024

Feature just released in https://pypi.org/project/zope.interface/6.2/.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants