Remove usage of sys.version_info #449

rahulporuri · 2019-02-03T11:00:00Z

~~NOTE : There is still one place where sys.version_info~~
I don't see any uses of python_version
but I'm not sure if we want to/can remove it completely.

Replace it's usage with six.PY2/six.PY3 instead
and remove outdated pieces of code which added workarounds for
python < 2.7 and < 3.4

With this PR, #364 can be closed, as 2to3 have been replaced and
sys.version_info usage has been removed.

and replace it's usage with six.PY2/six.PY3 instead and remove outdated pieces of code which added workarounds for python < 2.7 and < 3.4 modified: traits/_py2to3.py modified: traits/adaptation/adaptation_manager.py modified: traits/etsconfig/tests/test_etsconfig.py modified: traits/has_traits.py modified: traits/tests/test_float.py modified: traits/tests/test_list.py modified: traits/tests/test_traits.py modified: traits/trait_handlers.py modified: traits/trait_types.py modified: traits/traits.py

codecov-io · 2019-02-03T11:08:50Z

Codecov Report

Merging #449 into master will increase coverage by 0.66%.
The diff coverage is 87.09%.

@@            Coverage Diff             @@
##           master     #449      +/-   ##
==========================================
+ Coverage   64.71%   65.37%   +0.66%     
==========================================
  Files          44       44              
  Lines        7130     7053      -77     
  Branches     1418     1410       -8     
==========================================
- Hits         4614     4611       -3     
+ Misses       2092     2021      -71     
+ Partials      424      421       -3

Impacted Files	Coverage Δ
traits/trait_base.py	`57.97% <ø> (-0.61%)`	⬇️
traits/api.py	`87.87% <100%> (ø)`	⬆️
traits/has_traits.py	`67.8% <100%> (-0.03%)`	⬇️
traits/traits.py	`60.19% <100%> (ø)`	⬆️
traits/adaptation/adaptation_manager.py	`98.29% <100%> (ø)`	⬆️
traits/trait_handlers.py	`63.2% <100%> (ø)`	⬆️
traits/_py2to3.py	`80% <100%> (+37.81%)`	⬆️
traits/trait_types.py	`62.91% <69.23%> (+0.04%)`	⬆️
traits/etsconfig/etsconfig.py	`63.58% <0%> (+6.17%)`	⬆️

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 ef5313d...dd9a986. Read the comment docs.

traits/_py2to3.py

traits/adaptation/adaptation_manager.py

traits/etsconfig/tests/test_etsconfig.py

traits/has_traits.py

traits/tests/test_list.py

traits/tests/test_traits.py

mdickinson

This is great; thank you! A few minor change requests. In particular, we should avoid the use of six.PY3, since it'll be false for Python >= 4.0, which almost certainly isn't what we want. (Note: there may be a better constant in some future version of six. I wouldn't recommend using PY34: it's not clear to readers what it means, and judging by the linked issue there's a possibility it could change in the future.)

- replace use of _py2to3.assertCountEqual with six.assertCountEqual - reverse the order of if/else statements to check for six.PY2 instead of six.PY3 - reorder import

rahulporuri · 2019-02-05T09:33:19Z

the CI fails with two traitsui errors. this is because traitsui uses the str_find and str_rfind aliases defined in the _py2to3 private module. See PR enthought/traitsui#496 which moves the two alises to traitsui.

See the traceback below :

======================================================================
ERROR: test_auto_set_default (traits.tests.test_editor_factories.TestMultiLineEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traits\tests\test_editor_factories.py", line 34, in test_auto_set_default
    a = multi_line_text_editor(auto_set=False)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traits\traits.py", line 144, in multi_line_text_editor
    from traitsui.api import TextEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\api.py", line 36, in <module>
    from .editors.api import ArrayEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\__init__.py", line 23, in <module>
    from .api import ArrayEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\api.py", line 21, in <module>
    from .boolean_editor import BooleanEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\boolean_editor.py", line 35, in <module>
    from .text_editor import ToolkitEditorFactory as EditorFactory
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\text_editor.py", line 67, in <module>
    class ToolkitEditorFactory(EditorFactory):
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\text_editor.py", line 115, in ToolkitEditorFactory
    '|options:[Options]>'])
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\view.py", line 339, in __init__
    self.set_content(*values)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\view.py", line 354, in set_content
    content.append(Group(*value))
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\group.py", line 207, in __init__
    self._parse(value)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\group.py", line 354, in _parse
    value = self._parse_style(value)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\view_element.py", line 180, in _parse_style
    value = self._split('style', value, ';', _py2to3.str_rfind, 1, 0)
AttributeError: 'module' object has no attribute 'str_rfind'
======================================================================
ERROR: test_enter_set_default (traits.tests.test_editor_factories.TestMultiLineEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traits\tests\test_editor_factories.py", line 40, in test_enter_set_default
    a = multi_line_text_editor(enter_set=True)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traits\traits.py", line 144, in multi_line_text_editor
    from traitsui.api import TextEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\api.py", line 36, in <module>
    from .editors.api import ArrayEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\__init__.py", line 23, in <module>
    from .api import ArrayEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\api.py", line 21, in <module>
    from .boolean_editor import BooleanEditor
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\boolean_editor.py", line 35, in <module>
    from .text_editor import ToolkitEditorFactory as EditorFactory
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\text_editor.py", line 67, in <module>
    class ToolkitEditorFactory(EditorFactory):
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\editors\text_editor.py", line 115, in ToolkitEditorFactory
    '|options:[Options]>'])
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\view.py", line 339, in __init__
    self.set_content(*values)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\view.py", line 354, in set_content
    content.append(Group(*value))
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\group.py", line 207, in __init__
    self._parse(value)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\group.py", line 354, in _parse
    value = self._parse_style(value)
  File "C:\Users\appveyor\.edm\envs\traits-test-2.7\lib\site-packages\traitsui\view_element.py", line 180, in _parse_style
    value = self._split('style', value, ';', _py2to3.str_rfind, 1, 0)
AttributeError: 'module' object has no attribute 'str_rfind'
----------------------------------------------------------------------

rahulporuri · 2019-02-05T09:34:23Z

@mdickinson I've chosen to move the two aliases to traitsui. Is that okay?

This reverts commit cd320a6.

and their usage in traitsui. document the PR that removes the use of the private module in traitsui. modified: traits/_py2to3.py

rahulporuri · 2019-02-05T10:31:16Z

@mdickinson what about the python_version derived from sys.version_info in traits.trait_base?

mdickinson · 2019-02-05T11:06:12Z

what about the python_version derived from sys.version_info in traits.trait_base?

That should definitely go. I think the UUID change in this PR removes its only use, right?

Technically this isn't private, so there's a possibility that a 3rd party package is importing this, but I doubt that's the case. And expressing a package version as a float is a horrible, no-good, broken thing to do, especially if there's a prospect of minor version numbers larger than 9.

modified: traits/api.py modified: traits/trait_base.py modified: traits/trait_types.py

rahulporuri · 2019-02-05T11:17:23Z

That should definitely go. I think the UUID change in this PR removes its only use, right?

Yep. The only place it was being used was in the UUID trait definition.

traits/api.py

mdickinson

Changes LGTM. Thank you!

modified: CHANGES.rst

rahulporuri requested a review from mdickinson February 3, 2019 11:00

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/_py2to3.py Show resolved Hide resolved

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/_py2to3.py Outdated Show resolved Hide resolved

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/adaptation/adaptation_manager.py Outdated Show resolved Hide resolved

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/etsconfig/tests/test_etsconfig.py Show resolved Hide resolved

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/has_traits.py Outdated Show resolved Hide resolved

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/tests/test_list.py Outdated Show resolved Hide resolved

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/tests/test_traits.py Outdated Show resolved Hide resolved

mdickinson requested changes Feb 5, 2019

View reviewed changes

Poruri Sai Rahul added 2 commits February 5, 2019 14:17

CLN : remove aliases for str.find, str.rfind

cd320a6

CLN : address review comments

b0fdc54

- replace use of _py2to3.assertCountEqual with six.assertCountEqual - reverse the order of if/else statements to check for six.PY2 instead of six.PY3 - reorder import

rahulporuri mentioned this pull request Feb 5, 2019

Remove use of traits._py2to3 module enthought/traitsui#496

Merged

Poruri Sai Rahul added 2 commits February 5, 2019 15:36

Revert "CLN : remove aliases for str.find, str.rfind"

1fd2b8b

This reverts commit cd320a6.

DOC : add note about str.find/str.rfind in _py2to3

f830e98

and their usage in traitsui. document the PR that removes the use of the private module in traitsui. modified: traits/_py2to3.py

CLN : remove python_version

d718a5c

modified: traits/api.py modified: traits/trait_base.py modified: traits/trait_types.py

mdickinson reviewed Feb 5, 2019

View reviewed changes

traits/api.py Show resolved Hide resolved

mdickinson approved these changes Feb 5, 2019

View reviewed changes

DOC : update changelog

dd9a986

modified: CHANGES.rst

mdickinson merged commit d5bc4fa into master Feb 5, 2019

mdickinson deleted the ref/remove-sys-version-info-usage branch February 5, 2019 12:50

rahulporuri mentioned this pull request Feb 5, 2019

Remove use of 2to3 for Python 3 support. #364

Closed

rahulporuri added this to the After 5.0.0 release milestone Apr 15, 2019

mdickinson mentioned this pull request Apr 18, 2019

5.1.0 release broke Traits UI code using _py2to3 #471

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove usage of sys.version_info #449

Remove usage of sys.version_info #449

rahulporuri commented Feb 3, 2019 •

edited

Loading

codecov-io commented Feb 3, 2019 •

edited

Loading

mdickinson left a comment

rahulporuri commented Feb 5, 2019

rahulporuri commented Feb 5, 2019

rahulporuri commented Feb 5, 2019

mdickinson commented Feb 5, 2019

rahulporuri commented Feb 5, 2019

mdickinson left a comment

Remove usage of sys.version_info #449

Remove usage of sys.version_info #449

Conversation

rahulporuri commented Feb 3, 2019 • edited Loading

codecov-io commented Feb 3, 2019 • edited Loading

Codecov Report

mdickinson left a comment

Choose a reason for hiding this comment

rahulporuri commented Feb 5, 2019

rahulporuri commented Feb 5, 2019

rahulporuri commented Feb 5, 2019

mdickinson commented Feb 5, 2019

rahulporuri commented Feb 5, 2019

mdickinson left a comment

Choose a reason for hiding this comment

rahulporuri commented Feb 3, 2019 •

edited

Loading

codecov-io commented Feb 3, 2019 •

edited

Loading