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

silx.gui.utils.glutils: Fixed isOpenGLAvailable #3356

Merged
merged 6 commits into from
Jan 25, 2021

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Jan 22, 2021

Should fix issues with import when running isOpenGLAvailable. PR 3184 was apparently not enough.

It still needs testing.
Alternative can be to import subprocess lazily in def _runtimeOpenGLCheck(version)

Related to #3182

@vallsv
Copy link
Contributor

vallsv commented Jan 22, 2021

Ah ok i see.

But what is the part of the code relying on concurrent.futures?

That's internal to python itself?

@vallsv
Copy link
Contributor

vallsv commented Jan 22, 2021

Oh i remember now... Was the relative import or our concurrent module. No luck.

@t20100
Copy link
Member Author

t20100 commented Jan 22, 2021

It doesn't fix the issue, that's a weird issue.

@vallsv
Copy link
Contributor

vallsv commented Jan 22, 2021

To me the good way to handle it is to use -m with the module name.

Relative modules will just be relative, as we expect.

@vallsv
Copy link
Contributor

vallsv commented Jan 22, 2021

Here is traceback from a beamline

ERROR 2021-01-13 15:03:00,611 flint.output: Traceback (most recent call last):
ERROR 2021-01-13 15:03:00,611 flint.output:   File "/users/blissadm/conda/miniconda/envs/bliss_dev/lib/python3.7/site-packages/silx/gui/utils/glutils.py", line 37, in <module>
ERROR 2021-01-13 15:03:00,611 flint.output:     import subprocess
ERROR 2021-01-13 15:03:00,611 flint.output:   File "/users/blissadm/conda/miniconda/envs/bliss_dev/lib/python3.7/subprocess.py", line 50, in <module>
ERROR 2021-01-13 15:03:00,611 flint.output:     import signal
ERROR 2021-01-13 15:03:00,611 flint.output:   File "/opt/bliss/conda/miniconda/envs/bliss_dev/lib/python3.7/site-packages/silx/gui/utils/signal.py", line 32, in <module>
ERROR 2021-01-13 15:03:00,611 flint.output:     from silx.gui.utils import concurrent
ERROR 2021-01-13 15:03:00,611 flint.output:   File "/users/blissadm/conda/miniconda/envs/bliss_dev/lib/python3.7/site-packages/silx/gui/utils/concurrent.py", line 35, in <module>
ERROR 2021-01-13 15:03:00,611 flint.output:     from concurrent.futures import Future
ERROR 2021-01-13 15:03:00,611 flint.output:   File "/opt/bliss/conda/miniconda/envs/bliss_dev/lib/python3.7/site-packages/silx/gui/utils/concurrent.py", line 35, in <module>
ERROR 2021-01-13 15:03:00,611 flint.output:     from concurrent.futures import Future
ERROR 2021-01-13 15:03:00,611 flint.output: ModuleNotFoundError: No module named 'concurrent.futures'; 'concurrent' is not a package

No idea why they don't complain anymore.

@t20100
Copy link
Member Author

t20100 commented Jan 25, 2021

-m fixes the issue, thanks @vallsv for the idea.
There is still an issue when run through bootstrap.py though.

@t20100
Copy link
Member Author

t20100 commented Jan 25, 2021

Updated with an altenative: copy glutils.py in a temporary folder before running it as a script (this could be changed by having a glutils/__init__.py module and a separate glutils/_script.py for what is run as a subprocess).
From what I tested, it does not seems to change the duration of the test.

This avoids:

  • Running a script in the silx.gui.utils folder (which cause python's signal and concurrent packages to be overridden by those of silx.gui.utils)
  • Using -m which fails when run through bootstrap.py

I also added -s -S option to preserve as much as possible sys.path from calling process.

Still need to be tested on macos and windows

@vallsv
Copy link
Contributor

vallsv commented Jan 25, 2021

Any idea why the bootstrap is not working with -m? Souds like there is something weird with, or it just about our own protections?

@t20100
Copy link
Member Author

t20100 commented Jan 25, 2021

From my current understanding, bootstrap patches sys.path and this is patch is not done for the subprocess so you end-up with the root of the project in sys.path.
One option would be in bootstrap to change the current working directory when it is the root of the project, but that has other side-effects.

I'll still dig a bit, I am not happy with the temporary file, but for now it is what I found that works in most conditions.

@t20100
Copy link
Member Author

t20100 commented Jan 25, 2021

One last proposition (the best from my point of view): move glutils.py to glutils/__init__.py and run it as a script in the subprocess, so there is no conflict with what's in the script directory.

Works fine, no need for temporary file.

I added -s and -S python options to keep sys.path as it is in the calling process.

Ready for review.

@vallsv vallsv merged commit 793c25b into silx-kit:master Jan 25, 2021
@vallsv
Copy link
Contributor

vallsv commented Jan 25, 2021

Sounds like a cheat, but it's fine for me.

@t20100
Copy link
Member Author

t20100 commented Jan 25, 2021

Yes, I would have preferred to use -m but it fails with bootstrap.py... unless we move the sources in a sub-folder (#3357)

@t20100 t20100 deleted the fix-isOpenGLAvailable branch January 25, 2021 16:14
t20100 added a commit to t20100/silx that referenced this pull request Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants