Skip to content

FIX: Use trait_set instead of set#82

Merged
mdickinson merged 2 commits into
enthought:masterfrom
larsoner:dep
Jan 25, 2019
Merged

FIX: Use trait_set instead of set#82
mdickinson merged 2 commits into
enthought:masterfrom
larsoner:dep

Conversation

@larsoner
Copy link
Copy Markdown
Contributor

Fixes warnings of the form:

apptools/preferences/preferences_helper.py:166: DeprecationWarning: use "HasTraits.trait_set" instead
    self.set(trait_change_notify=notify, **traits_to_set)

Let me know if this needs to be a try: self.trait_set(...); except AttributeError: self.set(...) for backward compat and I can change it.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 23, 2019

Codecov Report

Merging #82 into master will decrease coverage by 0.31%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
- Coverage   40.77%   40.46%   -0.32%     
==========================================
  Files         240      240              
  Lines        9044     8910     -134     
  Branches     1197     1157      -40     
==========================================
- Hits         3688     3605      -83     
+ Misses       5212     5159      -53     
- Partials      144      146       +2
Impacted Files Coverage Δ
apptools/preferences/ui/widget_editor.py 0% <0%> (ø) ⬆️
apptools/template/test/scatter_plot.py 0% <0%> (ø) ⬆️
apptools/preferences/preference_binding.py 92.45% <100%> (ø) ⬆️
apptools/preferences/preferences_helper.py 87.87% <100%> (ø) ⬆️
apptools/sweet_pickle/versioned_unpickler.py 77.52% <0%> (-4.57%) ⬇️
apptools/persistence/versioned_unpickler.py 0% <0%> (-0.77%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0b6335...c6de8f8. Read the comment docs.

@mdickinson
Copy link
Copy Markdown
Member

Thanks for this. A quick "grin "\(.set\(" shows me a couple other suspicious uses of .set besides this one (though they're obviously not being exercised by the test suite. Grr):

./apptools/preferences/preference_binding.py:
  142 :             self.obj.set(trait_change_notify=notify, **traits)
./apptools/preferences/ui/widget_editor.py:
   80 :         return self.set(**traits)
./apptools/template/test/scatter_plot.py:
  195 :         plot.set(  padding_left   = 50,

Are you interested in fixing any of these in this PR? If so, I'll wait, but if not, this looks good to merge.

Let me know if this needs to be a try: self.trait_set(...); except AttributeError: self.set(...) for backward compat and I can change it.

No, no need; I haven't checked what ancient version of Traits would need set rather than trait_set, but for sure it's one we don't care about any more. We've been consistently replacing .set with .trait_set in other libraries.

@larsoner
Copy link
Copy Markdown
Contributor Author

@mdickinson done

@mdickinson
Copy link
Copy Markdown
Member

@larsoner Beautiful! Thank you.

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