Remove conditional traitsui import (ViewElement)#1033
Conversation
|
This change means that Traits 6.1.0 will effectively require TraitsUI 7.0.0 or later. While that was definitely the intention of #932, I wonder whether this is going to cause issues for projects. OTOH, if you're upgrading to Traits 6.1.0, it's reasonable to expect that you're also upgrading to the latest TraitsUI. Let's do it and see if anyone screams. We just need to be sure to document that the Traits 6.1.0 / TraitsUI 6.x.0 combination won't work. |
| def test_view_element_superclass(self): | ||
| from traitsui.api import ViewElement | ||
|
|
||
| self.assertIsInstance(ViewElement(), AbstractViewElement) |
There was a problem hiding this comment.
Possible overkill, but it might be worth testing for issubclass(ViewElement, AbstractViewElement) as well. That rules out ViewElement becoming a factory function at some future point, but I think that's OK.
There was a problem hiding this comment.
Well, in the long run, we don't want this test class here at all. The TraitsUI test suite is the right place to check that ViewElement behaves appropriately.
There was a problem hiding this comment.
Is it useful to enforce this limitation on ViewElement? From what I see, Traits always uses isinstance check, so it should work perfectly fine even if ViewElement becomes a factory function. Unless I'm missing something
There was a problem hiding this comment.
Agreed: all Traits cares about is that the view is an instance of AbstractViewElement, so I think it's enough to check just that here.
And again, really this check should be in TraitsUI: if there isn't a test for this in TraitsUI, we should add one. (And once it's there, we can remove this one.)
|
CI won't pass until EDM has TraitsUI 7 |
|
Ah. The CI is not going to pass until we have TraitsUI 7.0 in EDM. :-( We'll have to leave this open until that happens. FTR, the buildsystem PR is here: https://github.com/enthought/buildsystem/pull/3091 |
|
TraitsUI and Pyface 7.0 are now available in EDM. Closing and re-opening to trigger new CI runs. |
|
Houston, we have a problem: we're no longer building EDM packages for Python 3.5. There's an open issue somewhere to extend CI beyond Python 3.6 anyway (perhaps by using plain-old |
|
Opened #1044 to drop CI runs on Python 3.5. Once that's merged, this PR should be good to merge, too. |
|
I merged master in for #1044. Will merge this PR once the CI passes. |
3 participants
Closes #932
This import is no longer needed as
traitsuitakes care of registeringViewElementasAbstractViewElementsubclass, so it is removed. Also added a test to verify thatViewElementimported fromtraitsui.apiis indeed a subclass ofAbstractViewElement.Checklist
[ ] Update API reference (docs/source/traits_api_reference)[ ] Update User manual (docs/source/traits_user_manual)[ ] Update type annotation hints intraits-stubs