Skip to content

Deprecate get and set methods on HasTraits#190

Merged
mdickinson merged 8 commits into
enthought:masterfrom
sjagoe:remove-get-set
Dec 18, 2014
Merged

Deprecate get and set methods on HasTraits#190
mdickinson merged 8 commits into
enthought:masterfrom
sjagoe:remove-get-set

Conversation

@sjagoe
Copy link
Copy Markdown
Contributor

@sjagoe sjagoe commented May 9, 2014

@rkern Here is a new PR replacing #124. It also marks get and set as deprecated with tests to assert that they log the deprecation warning.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.19%) when pulling a222f20 on sjagoe:remove-get-set into 83eb368 on enthought:master.

@mdickinson
Copy link
Copy Markdown
Member

@sjagoe: RTM?

@mdickinson
Copy link
Copy Markdown
Member

I get one failure on this branch:

======================================================================
FAIL: test_trait_set_deprecated (traits.tests.test_trait_get_set.TestTraitGet)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/mdickinson/Enthought/Source/meta-geophysics/meta-ETS/traits/traits/tests/test_trait_get_set.py", line 80, in test_trait_set_deprecated
    self.assertEqual(len(handler.records), 1)
AssertionError: 0 != 1

@mdickinson
Copy link
Copy Markdown
Member

@sjagoe: How on earth did you bamboozle Travis into passing?!

@mdickinson
Copy link
Copy Markdown
Member

Okay, so the issue is that this test will only pass if HasTraits.set has not been called previously. And some recent change to TraitsUI introduced a HasTraits.set operation, which is triggered by a Traits import. Below is the (partial) stack trace leading to that operation.

I think the deprecation part of this PR should be revisited after dealing with PR #196.

  File "traits/tests/test_trait_get_set.py", line 21, in <module>
    from traits.testing.unittest_tools import unittest
  File "traits/testing/unittest_tools.py", line 19, in <module>
    from traits.api import (Any, Event, HasStrictTraits, Instance, Int, List,
  File "traits/api.py", line 107, in <module>
    from traitsui import view_elements
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traitsui/view_elements.py", line 37, in <module>
    from .view_element import ViewElement
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traitsui/view_element.py", line 34, in <module>
    from .ui_traits import (ATheme, AnObject, DockStyle, EditorStyle, ExportType,
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traitsui/ui_traits.py", line 371, in <module>
    class HasMargin ( TraitType ):
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traitsui/ui_traits.py", line 380, in HasMargin
    default_value = Margin( 0 )
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traitsui/ui_traits.py", line 327, in __init__
    self.set( left = left, right = right, top = top, bottom = bottom )

@mdickinson
Copy link
Copy Markdown
Member

#220 changes the deprecated decorator and adds a new assertDeprecated assertion to UnittestTools. If that gets merged, testing the deprecation in this PR should be easier.

@sjagoe sjagoe changed the title Remove get set Deprecate get and set methods on HasTraits Dec 18, 2014
@sjagoe
Copy link
Copy Markdown
Contributor Author

sjagoe commented Dec 18, 2014

Updated with tests for deprecation warnings from #220.

@mdickinson
Copy link
Copy Markdown
Member

Thanks. We still need a CHANGES.txt entry, but apart from that this LGTM.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) when pulling 9bc1db1 on sjagoe:remove-get-set into 0c301d2 on enthought:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.06%) when pulling 3f8cd30 on sjagoe:remove-get-set into 0c301d2 on enthought:master.

@mdickinson
Copy link
Copy Markdown
Member

Merging. Thanks!

mdickinson added a commit that referenced this pull request Dec 18, 2014
Deprecate get and set methods on HasTraits
@mdickinson mdickinson merged commit 4bd90e9 into enthought:master Dec 18, 2014
@sjagoe sjagoe deleted the remove-get-set branch December 18, 2014 16:56
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.

3 participants