-
Notifications
You must be signed in to change notification settings - Fork 54
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
napari test requirement #51
Conversation
Registering napari-conftest as a plugin in setup.py makes a hard-requirement on napari. This PR moves the requirement to tests only and registers the pytest plugin at runtime. With thanks to @Czaki see #napari/napari#1661
@tlambert03 : somewhere you had pointed me to a Qt-testing-in-GitHub-Actions solution that you had come up with. Can you point me to that pointer? |
4a41e1f
to
cb29a35
Compare
with thanks to @Czaki
f2c4142
to
583760c
Compare
12d64bc
to
f764994
Compare
be56bab
to
8f6d31d
Compare
Import suggestion from @Czaki in #67 solved the issue, though I think it seems un-pytest-plugin-y. Now hitting a missing
|
This error with |
I have already seen a problem with python 3.7 Pyside2 on Linux. I check in my code and skip this case in tests. |
Thanks, @Czaki. Yeah, don't think I'll try to tackle this segfault:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few remarks.
- {os: windows-latest, python_Version: '3.6', toxenv: 'py36-PyQt5'} | ||
- {os: windows-latest, python_Version: '3.6', toxenv: 'py36-PySide2'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no py36-PyQt5 and py36-PySide2 entries in tox files.
tox.ini
Outdated
PySide2: PySide2!=5.15.0 | ||
|
||
commands = | ||
pip install .[napari] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pip install .[napari]
is not necsssary
Thanks, @Czaki, but still getting failures on linux && (mac/py.39) |
As I see it fails on installed
maybe disable python 3.9 test with napari. |
Agreed! That has only started happening today. As for numcodecs & the wheel, yes, the other alternative would be to go back to conda. What do you think? |
The big problem with conda is that it goes with preinstalled qt in the old version (5.9) which breaks napari tests. Personally, I will skip testing this for python 3.9 and open issue on If I good remember there is problem with vispy which install on python 3.9 but will not run (maybe it was solved? @sofroniewn @jni did you know?) |
Understood. Thanks for the heads up. Unfortunate then that we tried to do all of our workshops for I2K with conda.
Wow. Ok. I recently had issues using anything other than 3.9 (at least on Mac Big Sur). I guess 3.8 is the sweet spot then. |
that was solved in vispy/vispy#1914 and released in vispy 0.6.6 |
There is work on enabling python 3.8 on ARM BigSure. pypa/wheel#387 (comment)
It does not break napari itself but breaks napari tests. Also It is possible to uninstall but it increases the complication of the process. Maybe it is simpler than I think. I'm not very familiar with conda, so maybe I chose the wrong way. Maybe someone from napari maintainers could help here. |
Ok. As it stands, we're green. In summary:
I think it's still enough testing for the purposes of ome-zarr, but help finding reliable CI testing across the spectrum would be appreciated, and I'd certainly also be interested in doing that with conda (both within CI and outwith). However, that can all be handled elsewhere. |
P.S.
We did have workshop attendees who had trouble with napari+conda. |
@@ -0,0 +1,8 @@ | |||
black | |||
napari |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Czaki, for a follow-up, thoughts on removing this if we're handling it via the "extras = " setting?
Merging. I'll be opening an immediate follow-up to activate coverage testing. Happy to make adjustments there. |
Registering napari-conftest as a plugin in setup.py
makes a hard-requirement on napari. This PR moves the
requirement to tests only and registers the pytest
plugin at runtime.
With thanks to @Czaki
see #napari/napari#1661
Note: that the environment.yml file is currently not tested by this PR and I in fact needed to actively remove
tox-conda
to get things to work. We will likely need to either remove it (in favor of an ome-zarr package on conda?) or add in separate testing.