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

Add glue-core to dev-deps and Python 3.13 envs to CI; compatibility with glue 1.22 BaseViewer #26

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dhomeier
Copy link
Contributor

@dhomeier dhomeier commented Sep 30, 2024

Description

Also switching dev: numpy and dev: astropy to use nightly wheels.
Updating the minimum Python version for dev as required by Astropy 7.x (note: conflicting jobs would no longer fail with astropy>=0.0.dev0, but silently downgrade to 6.1.x!).

Will (hopefully) fix #23

@dhomeier
Copy link
Contributor Author

dhomeier commented Oct 1, 2024

Checksum error for pyqt65 is hopefully transient; test_unit_conversion_limits failing in dev as a result of glue-viz/glue#2513 – arguably returning now the correct y limits [2.7, 3.3] for constant data == 3.0, so we will need to check the version there somehow.

@dhomeier dhomeier closed this Feb 11, 2025
@dhomeier dhomeier reopened this Feb 11, 2025
@dhomeier dhomeier force-pushed the glue-dev+313 branch 3 times, most recently from 79c118c to eb7d33e Compare February 12, 2025 17:57
@dhomeier
Copy link
Contributor Author

dhomeier commented Feb 12, 2025

@astrofrog, @iisakkirotko, it seems we have a major regression with glue-core dev and now 1.22 on failing viewer.close() calls, either in a teardown_method or called explicitly. This seems obviously related to the introduction of BaseViewer.close() in glue-viz/glue#2520, but what was happening there before when the method did not exist at all? Any advice much appreciated.

@dhomeier
Copy link
Contributor Author

The problem is, as you suspected @astrofrog, for all viewers inheriting from both glue.viewers.common.viewer.Viewer and glue_qt.viewers.common.base_widget.BaseQtViewerWidget BaseViewer.close now shadows the one from base_widget.

Overriding that manually in DataViewer seems to resolve all failures, but do you know of a more elegant way to make BaseQtViewerWidget take precedence?

@astrofrog
Copy link
Member

Overriding that manually in DataViewer seems to resolve all failures, but do you know of a more elegant way to make BaseQtViewerWidget take precedence?

No, I think the approach here is probably the best. Maybe you could add a comment to explain for posterity why we need to do this?

@dhomeier
Copy link
Contributor Author

Ok, was wondering if some super foo was in place instead.
That leaves the 5 test_python_export.py in the different viewers, working better in glue-core already. But maybe they're better removed in a separate PR?

@dhomeier
Copy link
Contributor Author

I have moved the enchant brew library to macOS docs only, since it now fails to install on macos-13.
The update to pyenchant is strictly only needed on macOS as well, but since it's an rc should be ok for all archs (would have to look up how to restrict it in tox.ini otherwise).

@dhomeier dhomeier changed the title Update CI to include glue-core to dev-deps; add Python 3.13 envs Add glue-core to dev-deps and Python 3.13 envs to CI; compatibility with glue 1.22 BaseViewer Feb 13, 2025
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.

Latest repository release doesn't show up on PyPI
2 participants