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

MAINT: replace deprecated version check for traitsui #1746

Merged
merged 3 commits into from
Apr 21, 2023

Conversation

homosapien-lcy
Copy link
Contributor

@homosapien-lcy homosapien-lcy commented Apr 21, 2023

In traitsui, traitsui.version has been deprecated and should be replaced with importlib.metadata.version('traitsui'). The deprecation warning raised have also caused test_edit_not_given in traits/traits/tests/test_configure_traits.py to fail as mentioned in #1745 . Since the version check is a temporary check to address the dependency of traits on traits ui and is no longer need, the current PR fixed this issue by removing all version checks related to _traitsui_helpers.py. closes #1745

Checklist

  • Tests (it's using the same test as before)
  • Update API reference (docs/source/traits_api_reference) (only replacing a deprecated version check, no change in behavior)
  • Update User manual (docs/source/traits_user_manual) (only replacing a deprecated version check, no change in behavior)
  • Update type annotation hints in stub files (only replacing a deprecated version check, no change in behavior)

@homosapien-lcy
Copy link
Contributor Author

homosapien-lcy commented Apr 21, 2023

I noticed a new test error happened:

FAIL: test_check_version_error (traits.util.tests.test_traitsui_helpers.TestTraitsUIHelper)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.10.11/x64/lib/python3.10/site-packages/traits/util/tests/test_traitsui_helpers.py", line 25, in test_check_version_error
    with self.assertRaises(RuntimeError) as exception_context:
AssertionError: RuntimeError not raised

So is this deprecation error supposed to be there to be check by another test (in MacOS, this deprecation error will cause the full unittest to fail)?

@dpinte
Copy link
Member

dpinte commented Apr 21, 2023

The issue is that the test still relies on the deprecated way to set the version:

with mock.patch("traitsui.__version__", "6.1.2"):

You'll need to adapt the mock to properly change the version.

@mdickinson
Copy link
Member

Thanks for looking at this, @homosapien-lcy. Actually, I think we can simply remove the version check and the test for it. This was an interim measure that was needed at some point in the past when Traits had a stronger dependence on TraitsUI than it does now.

@homosapien-lcy
Copy link
Contributor Author

Thanks for looking at this, @homosapien-lcy. Actually, I think we can simply remove the version check and the test for it. This was an interim measure that was needed at some point in the past when Traits had a stronger dependence on TraitsUI than it does now.

Got it, I remove all version checks for this PR

Copy link
Member

@mdickinson mdickinson left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@mdickinson mdickinson merged commit 60f918e into main Apr 21, 2023
27 checks passed
@mdickinson mdickinson deleted the fix_deprecated_version_check branch April 21, 2023 09:45
@mdickinson mdickinson mentioned this pull request Jul 10, 2023
mdickinson pushed a commit that referenced this pull request Jul 10, 2023
In traitsui, traitsui.__version__ has been deprecated and should be
replaced with importlib.metadata.version('traitsui'). The deprecation
warning raised have also caused test_edit_not_given in
traits/traits/tests/test_configure_traits.py to fail as mentioned in
#1745 . Since the version check is a temporary check to address the
dependency of traits on traits ui and is no longer need, the current PR
fixed this issue by removing all version checks related to
_traitsui_helpers.py.

Closes #1745

Co-authored-by: Chengyu Liu <cyliu@aus552cyliu.local>
@mdickinson mdickinson mentioned this pull request Jul 10, 2023
mdickinson pushed a commit that referenced this pull request Jul 26, 2023
In traitsui, traitsui.__version__ has been deprecated and should be
replaced with importlib.metadata.version('traitsui'). The deprecation
warning raised have also caused test_edit_not_given in
traits/traits/tests/test_configure_traits.py to fail as mentioned in
#1745 . Since the version check is a temporary check to address the
dependency of traits on traits ui and is no longer need, the current PR
fixed this issue by removing all version checks related to
_traitsui_helpers.py.

Closes #1745

Co-authored-by: Chengyu Liu <cyliu@aus552cyliu.local>
(cherry picked from commit 60f918e)
mdickinson pushed a commit that referenced this pull request Jul 26, 2023
In traitsui, traitsui.__version__ has been deprecated and should be
replaced with importlib.metadata.version('traitsui'). The deprecation
warning raised have also caused test_edit_not_given in
traits/traits/tests/test_configure_traits.py to fail as mentioned in
#1745 . Since the version check is a temporary check to address the
dependency of traits on traits ui and is no longer need, the current PR
fixed this issue by removing all version checks related to
_traitsui_helpers.py.

Closes #1745

Co-authored-by: Chengyu Liu <cyliu@aus552cyliu.local>
(cherry picked from commit 60f918e)
mdickinson added a commit that referenced this pull request Aug 7, 2023
This PR (against the maint/6.4 branch) is aimed at a 6.4.2 bugfix
release.

Key changes:

- Backport PR #1746 
- Backport PR #1749 
- Update copyright headers and bring CI back to working state
- Update changelog, bump version for release.

The PR is pretty horrendous to review en masse thanks to the changelog
updates, but it should be possible to review it commit by commit.

@corranwebster Would you have bandwidth for review at some point? (Feel
free to unassign if not.)

---------

Co-authored-by: homosapien-lcy <102019577+homosapien-lcy@users.noreply.github.com>
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.

test_configure_traits.py failed due to unexpected number of warnings in test_edit_not_given
3 participants