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

TraitsUI on Python 3.11 and PySide 6.4 #1994

Merged
merged 32 commits into from
Mar 24, 2023
Merged

TraitsUI on Python 3.11 and PySide 6.4 #1994

merged 32 commits into from
Mar 24, 2023

Conversation

corranwebster
Copy link
Contributor

@corranwebster corranwebster commented Mar 17, 2023

Currently seems to work with Pyface 7.x

This PR does the following:

  • removes setup.py and moves to a pyproject.toml system (includes deprecating traitsui.__version__)
  • updates for Python 3.11
  • updates for PySide 6.4
  • updates for Pyface 8.0
  • lots of messing around with the CI infrastructure
    • in particular adds a full set of tests on all supported platforms, Python versions and toolkit backends
    • adds some bleeding-edge tests
  • some drive-by fixes (eg. copy-paste issues with some Wx tests using Qt APIs)

Currently skipping tests failures on Wx due to failures: see #1998

Checklist

  • Add a news fragment if this PR is news-worthy for end users. (see docs/releases/README.rst)

@corranwebster
Copy link
Contributor Author

This PR will need the expected tests to be updated before it can be merged.

@corranwebster corranwebster marked this pull request as ready for review March 21, 2023 12:56
@corranwebster corranwebster marked this pull request as draft March 21, 2023 13:05
@corranwebster corranwebster marked this pull request as ready for review March 21, 2023 16:54
@mdickinson mdickinson self-requested a review March 21, 2023 16:59
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. A few minor comments - nothing remotely blocking.

run: cd testdir && python -X faulthandler -m unittest discover -v traitsui
if: matrix.os != 'ubuntu-latest'

# Wx Tests are turned off for now
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth deleting the code here and moving it to a separate workflow that we can run on demand (but that isn't run in response to PRs), just so that we have a way to check current status of wx builds?

run: edm run -- python etstool.py test --toolkit=${{ matrix.toolkit }}
working-directory: ets-demo
run: xvfb-run -a edm run -- python etstool.py test --toolkit=${{ matrix.toolkit }}
working-directory: ets-demo
Copy link
Member

Choose a reason for hiding this comment

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

Nice: I keep forgetting that the working-directory setting exists.

pyproject.toml Outdated
[tool.black]
line-length = 79
target-version = ['py36', 'py37', 'py38']
skip-string-normalization = true
target-version = ['py38']
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick-level, but it's quite tempting to remove this altogether (unless versions of black that we currently care about fail without it). In the future, this shouldn't be needed any more - black will infer the relevant Python versions from the requires-python project metadata.

xref: psf/black#3219 (merged, but not yet released AFAICT)
xref: psf/black#751 (which I started reading because I was wondering whether we were supposed to be providing a single version or multiple versions here).

pyproject.toml Outdated
[tool.isort]
profile = 'black'
sections = ['FUTURE', 'STDLIB', 'THIRDPARTY', 'ENTHOUGHT', 'FIRSTPARTY', 'LOCALFOLDER']
known_third_party = ['wx', 'PyQt5', 'PyQt6', 'PySide2', 'PySide6', 'PIL', 'pygments', 'numpy']
Copy link
Member

Choose a reason for hiding this comment

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

Just an observation: maintaining the known_third_party list has been getting annoying on other projects, and isort doesn't seem to care much if it's omitted. (Essentially, anything not recognised gets classed as third party.)

sections = ['FUTURE', 'STDLIB', 'THIRDPARTY', 'ENTHOUGHT', 'FIRSTPARTY', 'LOCALFOLDER']
known_third_party = ['wx', 'PyQt5', 'PyQt6', 'PySide2', 'PySide6', 'PIL', 'pygments', 'numpy']
known_enthought = ['apptools', 'pyface', 'traits']
line_length = 79
Copy link
Member

Choose a reason for hiding this comment

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

Entirely non-actionable observation: it's really annoying that black wants a hyphen while isort wants an underscore for this option.

@corranwebster
Copy link
Contributor Author

If required tests pass we should be good to go - we don't expect PySide 6.4.3 to pass with Pyface < 8, and Pyface 8 hasn't been released yet, so those aren't required. They should go green with Pyface 8 being on PyPI - when that happens we will make them required.

@corranwebster corranwebster merged commit 2e224d6 into main Mar 24, 2023
@corranwebster corranwebster deleted the enh/remove-setup branch March 24, 2023 16:59
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.

2 participants