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

Deprecate the use of CHECK_INTERFACES for automatic interface checking. #1231

Merged
merged 5 commits into from
Jul 13, 2020

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Jun 26, 2020

[Builds on the commits in #1225; related to issue #1221, but doesn't completely resolve that issue]

Currently, if has_traits.CHECK_INTERFACES is nonzero, the @provides decorator performs interface checking at the point where the class is decorated.

This has a number of issues:

  • the machinery is awkward to use: typically, the call to provides happens at import time, so someone who wants to make use of this needs to be careful about import order, setting has_traits.CHECK_INTERFACES before the imports they want checked happen
  • there's significant processing happening at import time, which is not ideal
  • the interface checking performed doesn't match the isinstance check that Instance(ISomeInterface) now does at trait assignment time (pre Traits 6.0.0, it did match)
  • the machinery complicates the has_traits.py source, adding a circular dependency that has to be resolved via a local import

Longer term, if we do interface checking we likely want to do it statically.

This PR deprecates the interface-checking behaviour of provides, and issues a deprecation warning for any use of @provides with a nonzero CHECK_INTERFACES value. In a future Traits version (likely 7.0), we'd then remove the interface-checking call from provides altogether.

In a search of Traits-using code, I failed to find any code outside Traits that makes use of a non-zero CHECK_INTERFACES. I would expect the impact of this change to be negligible.

Checklist

  • Tests
  • Update API reference (docs/source/traits_api_reference) (Not relevant in this case)
  • Update User manual (docs/source/traits_user_manual)
  • Update type annotation hints in traits-stubs (Not relevant in this case)

@mdickinson mdickinson added this to the 6.2.0 release milestone Jun 26, 2020
traits/has_traits.py Outdated Show resolved Hide resolved
@mdickinson
Copy link
Member Author

mdickinson commented Jun 26, 2020

Updated after a master merge. Though now that #1165 is in, I'm wondering whether some of those comparison_mode=ComparisonMode.identity annotations can be removed. @kitchoi ?

Sorry. Wrong PR. This comment was meant for #1229

@mdickinson
Copy link
Member Author

Updated the description to add some rationale.

Copy link
Contributor

@kitchoi kitchoi left a comment

Choose a reason for hiding this comment

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

LGTM

(Operational bit: this needs #1225 to be merged first, and then master merged back in here.)

@mdickinson mdickinson merged commit 8af4012 into master Jul 13, 2020
@mdickinson mdickinson deleted the deprecate-provides-interface-checking branch July 13, 2020 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants