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

BUG: fix issue where designer doesn't load pydm widgets when pydm is installed from PyPI using pip #1132

Merged
merged 4 commits into from
Dec 10, 2024

Conversation

nstelter-slac
Copy link
Collaborator

@nstelter-slac nstelter-slac commented Nov 21, 2024

When pydm is installed from PyPI using pip, you should be able to set the env variable PYQTDESIGNERPATH to pip pydm install location so designer can load the pydm widgets.

But for PYQTDESIGNERPATH to be used to load pydm widgets, it needs to point to a dir containing the pydm_designer_plugin.py file. This file was put in the pydm repo's root dir. But pip only copies to the install locations python-package dirs (dir contains __init__.py), which for pydm is the /pydm subfolder of the root dir (which didn't contain the pydm_designer_plugin.py file).

The solution is to place the plugin file in the <pydm_root_dir>/pydm subdir so pip can grab it.

Also make conda install cmd in doc pin pyqt version.

@nstelter-slac nstelter-slac force-pushed the fix_designer_pip_install branch from 67d38d2 to 6c963cc Compare November 21, 2024 23:20
@nstelter-slac nstelter-slac changed the title BUG: fix issue where designer doesn't load pydm widgets when pydm is BUG: fix issue where designer doesn't load pydm widgets when pydm is installed from PyPI using pip Nov 21, 2024
installed from PyPI using pip

When pydm is installed using pip, you should be able to
set the env variable PYQTDESIGNERPATH to the pip pydm install location and then designer loads the pydm widgets.

But for PYQTDESIGNERPATH to be used to load pydm widgets, it must point to dir containing
pydm_designer_plugin.py file. This file was wrongly placed in the pydm repo
root dir. But pip only copies to the install location any python-package
dirs (dir with a __init__.py), which for pydm is the /pydm subfolder
of the root dir (which did not contain the pydm_designer_plugin.py file).

So solution is to place the plugin file in the /pydm subdir so pip
copies it to the install location, and so PYQTDESIGNERPATH can find the
file when set to the install location.

(conda installs are not affected, does not use pydm_designer_plugin.py
file)
@nstelter-slac nstelter-slac force-pushed the fix_designer_pip_install branch from 6c963cc to 95810d4 Compare November 21, 2024 23:37
@nstelter-slac
Copy link
Collaborator Author

nstelter-slac commented Nov 21, 2024

tested patch as follows:

// install everything except pydm in conda env
conda create -n pydm-pydm_test_pip_fix python=3.8 pyqt=5.12.3 pip numpy scipy six psutil pyqtgraph -c conda-forge
// install pydm from local repo on this patch
pip install pydm/
// find pip install location of pydm and set PYQTDESIGNERPATH
pip show pydm
export PYQTDESIGNERPATH=/home/nstelter/miniconda3/envs/pydm_test_pip_fix/lib/python3.8/site-packages/pydm
// confirm plugin file is in correct location
ls $PYQTDESIGNERPATH/pydm_designer_plugin.py
// launch designer and confirm pydm widgets load
designer

@nstelter-slac
Copy link
Collaborator Author

seems like this shouldn't effect conda installs, since they create their own pydm_designer_plugin.py file:

https://github.com/slaclab/pydm/blob/master/conda-recipe/build.sh#L15

@nstelter-slac nstelter-slac marked this pull request as ready for review November 21, 2024 23:48
I think people might miss the warning text and just copy the listed conda cmd,
so having this cmd just work could avoid some issues.
@YektaY
Copy link
Collaborator

YektaY commented Nov 22, 2024

This was the origin behind changing the installation guide to unpin pyqt: #999
Do you think it is more clear to have it pinned then just leaving a note that they need to pin to an older version of pyqt to be able to use pydm widgets in designer?

Since pyqt=5 is valid install option, just designer won't work with pydm
widgets.
Explicity list the cmd for pyqt=5.12.3, to avoid any confusion on how
to pin, and maybe to catch anyone's eyes just looking for install cmds.
@nstelter-slac nstelter-slac force-pushed the fix_designer_pip_install branch from 4c91ae3 to c4be6f3 Compare November 22, 2024 23:44
@nstelter-slac
Copy link
Collaborator Author

nstelter-slac commented Nov 22, 2024

ok agree now on having main listed conda cmd pinned to pyqt=5, thanks!

but what about adding an example cmd of pyqt=5.12.3 pinned in the 'warning' section: https://github.com/slaclab/pydm/pull/1132/files#diff-e285c1b9901cd202e89d9f37e3249a6fee524a65428895e1a45fd062cc0f9acaL29

  • explicitly shows how to pin the version
  • acts to catch the eyes of anyone just skimming docs for the correct install cmd (this happened to me b4, copy and pasted conda cmd but forgot to add pin)

@jbellister-slac
Copy link
Collaborator

acts to catch the eyes of anyone just skimming docs for the correct install cmd (this happened to me b4, copy and pasted conda cmd but forgot to add pin)

I think I actually prefer the pinned 5.12.3 version as the default command for that reason. Or at the very least the warning box appearing before the install command so the reader sees it first.

We can also remove the pin of python to 3.8 there as it should work at least from 3.7-3.10, and should be handled by our conda recipe anyway. Should update that recipe during the next release to ensure it is correct.

@nstelter-slac
Copy link
Collaborator Author

ok moved the warning box above the normal cmd, but normal cmd has no pyqt pin. imo this is best of both worlds.

@nstelter-slac nstelter-slac force-pushed the fix_designer_pip_install branch from 210c5d3 to b645c3d Compare December 3, 2024 03:19
@nstelter-slac
Copy link
Collaborator Author

@jbellister-slac for the python pin, should we pin 3.7-3.10? or think we should list most recent python (3.12)?
if so, maybe we should add automated test running on 3.12?

also in what way should conda recipe be updated? no python version is pinned there atm

Copy link
Collaborator

@jbellister-slac jbellister-slac left a comment

Choose a reason for hiding this comment

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

Oh I was looking over on conda forge:

https://github.com/conda-forge/pydm-feedstock/blob/main/recipe/meta.yaml

But yeah should update the version you were looking at first so they stay in sync, didn't realize there was no python version here. We could probably put a lower bound of at least 3.8 since even 3.7 has been end of life for a while now.

Running the tests against newer python versions also sounds good, but both of these things can be part of a different PR.

@nstelter-slac nstelter-slac force-pushed the fix_designer_pip_install branch 2 times, most recently from 2201a7e to dd25549 Compare December 10, 2024 19:45
…s likely to miss it. Also remove python 3.8 since 3.7-3.10 should work.
@nstelter-slac nstelter-slac force-pushed the fix_designer_pip_install branch from dd25549 to 91e2525 Compare December 10, 2024 19:47
@YektaY YektaY merged commit 136ed4a into slaclab:master Dec 10, 2024
19 checks passed
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.

Fix PYQTDESIGNERPATH mechanism for pydm pip installations?
3 participants