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

Module path discovery is broken #25

Closed
dihm opened this issue Jan 31, 2024 · 7 comments · Fixed by #26
Closed

Module path discovery is broken #25

dihm opened this issue Jan 31, 2024 · 7 comments · Fixed by #26

Comments

@dihm
Copy link
Contributor

dihm commented Jan 31, 2024

Issue initially raised in labscript-suite/labscript-suite#83. I'm not able to reproduce the issue raised there exactly, but I am getting some crazy stuff that just doesn't make any sense to me.

My test script

import desktop_app
import importlib_metadata
from pathlib import Path

modules = ['blacs', 'lyse', 'runmanager', 'runviewer']

for module in modules:
    print(f'\nChecking module {module}')
    print('\tget_packge_directory:', desktop_app.environment.get_package_directory(module))
    print('\tget_install_directory:', desktop_app.environment.get_install_directory(module))
    print('\tget_scripts_dir:', desktop_app.environment.get_scripts_dir(module))
    print('\tget_distribution_of_module:', desktop_app.environment.get_distribution_of_module(module), '\n')

for distribution in importlib_metadata.distributions():
    if distribution.files is None:
        continue
    for path in distribution.files:
        if Path(path).match('*lyse*'):
            print(path)

I generally see two types of outputs from this script depending on if things work or not.

  1. Working has zero none paths, and the distribution paths output includes lots of lyse/* type paths.
  2. Non-working give none on get_distribution_of_module. The only paths I get from the lyse search are
Cython/Compiler/AnalysedTreeTransforms.py
Cython/Compiler/__pycache__/AnalysedTreeTransforms.cpython-311.pyc
../../Scripts/lyse-gui.exe
../../Scripts/lyse.exe
__editable__.lyse-3.1.0rc2.dev84+g1f0c876.pth
__editable___lyse_3_1_0rc2_dev84_g1f0c876_finder.py
__pycache__/__editable___lyse_3_1_0rc2_dev84_g1f0c876_finder.cpython-311.pyc

I have a number of labscript installations I can try this script on throughout the lab, and I can't find a consistent thread.

  • Computer 1 (my dev computer, no hooked up to hardware, never run desktop-app on it, conda=23.11.0)
    • Python 3.11 environment (just installed)
      • Fails
    • Python 3.9 environment (installed a long time ago)
      • Had to update desktop-app so script could run, Fails
  • Computer 2 (active experiment, conda=23.7.3)
    • Python 3.10 environment (installed a few months ago)
      • This fails now, but clearly didn't when I installed just a few months ago since it has shortcuts
  • Computer 3 (old inactive experiment, conda=4.13.0)
    • Python 3.8 (old inactive experiment install, installed years ago)
      • This works now
    • Python 3.11 (just installed)
      • Fails
  • Computer 4 (another dev computer, with hardware, slightly active recently, conda=23.3.1)
    • Python 3.9 environment (installed a year ago)
      • Works

Best guess is that importlib_metadata.distributions reports differently depending on conda enviroment and python version. Now that I think about it, all of these installs are using the anaconda developer instructions, so those conda version (from the base environment) maybe aren't being used on the newer installs consistently (since setuptools-conda install conda locally to the environment as well).

@dihm
Copy link
Contributor Author

dihm commented Feb 15, 2024

@ispielma You mentioned that you successfully installed labscript from scratch just last week. Could I trouble you for some details? Python version, conda version, OS, installation method would be helpful.

@ispielma
Copy link

ispielma commented Feb 15, 2024

Sure. I did the pip install -e ... developer install on:

(1) my mac starting with a miniconda install with versions:

conda 24.1.0
pip 23.3.1
Python 3.12.1
MacOS 14.3

(2) A windows 10 PC starting with a miniconda install with versions:

conda 23.11.0
pip 23.3.1
Python 3.10.12
Windows 10.0.19044

In both cases the only issue was the one where I had to make the main git repo an upstream repository for each labscript repository on my computer (even though they were cloned from my fork) and then manually pull over all the version flags.

@dihm
Copy link
Contributor Author

dihm commented Feb 17, 2024

@ispielma Could you also say what pip version you have? It appears this has something to do with pip modifying its behavior when installing in editable mode around pip==22.

Edit: I believe this issue hints at the problem. It looks like pip isn't super happy with our dynamic versioning and it can slightly bork the installation such that desktop-app's heuristics for finding paths and packages breaks, but the installation imports and works. More research needed.

@ispielma
Copy link

Updated above, but in any case pip 23.3.1 on both systems.

@dihm
Copy link
Contributor Author

dihm commented Feb 26, 2024

I think I've found a fix for this issue, but it doesn't account for Ian's installation actually working. Short of it is pip==22 brought in support for PEP 660 handling of editable installations which breaks desktop-app's logic for finding the distribution name of the module. If that's true, Ian's installation shouldn't have worked. Now, this issue is only encountered if desktop-app is used to generate the GUI desktop shortcuts.

So just to confirm, @ispielma, did you use desktop-app to generate desktop shortcuts on both of those installations?

@ispielma
Copy link

I cannot confirm that the desktop shortcuts were generated on the mac install, but they are present on the PC (and I have used them for Lyse at least once). Generally I run everything from the command line, so I have not tested the shortcuts extensively.

@dihm
Copy link
Contributor Author

dihm commented Feb 26, 2024

Ah OK. That makes more sense. I suspect if you tried to run desktop-app to (re)create the shortcuts you'd see this broken behavior. I'll knock together a PR with the change that I think is necessary to sort this out.

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 a pull request may close this issue.

2 participants