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

Python submodules cannot call each other using absolute imports #138

Closed
caiw opened this issue Jan 26, 2024 · 5 comments
Closed

Python submodules cannot call each other using absolute imports #138

caiw opened this issue Jan 26, 2024 · 5 comments
Labels
🪲 bug Something isn't working 👷 design standards ❌ wontfix This will not be worked on

Comments

@caiw
Copy link
Member

caiw commented Jan 26, 2024

When running the toolbox I'm encountering the following error:

[cw04@login-j05 kymata-toolbox]$ ./submit_gridsearch.sh
Loading apptainer version:
1.0.3
Traceback (most recent call last):
  File "/imaging/projects/cbu/kymata/analyses/andy/kymata-toolbox/invokers/run_gridsearch.py", line 5, in <module>
    from kymata.datasets.data_root import data_root_path
ModuleNotFoundError: No module named 'kymata'

It seems that the reason is that it looks like in Python you can't use absolute module importing from within a submodule. I.e., because run_gridsearch.py is inside kymata/, it doesn't know what kymata is. When running in an IDE, this is solved automatically using option 2 below.

The two solutions seem to be:

  1. use relative imports everywhere (e.g. in run_gridsearch.py, instead of beginning from kymata.datasets.data_root import data_root_path it would begin from .datasets.data_root import data_root_path
  2. manually add the toolbox to the PYTHONPATH variable.

Which is best?

2 is much easier, but requires some documentation, and could cause subtle errors in case there are multiple copies of the toolbox around. It is also recommended against
1 is more "correct", and is probably what we should do at some point in preparation for #4.

@caiw caiw added 🪲 bug Something isn't working 👷 design standards labels Jan 26, 2024
@caiw
Copy link
Member Author

caiw commented Jan 26, 2024

Qustion: why doesn't this cause an issue with the CI tests?
Or in other words, how could we add a test to the CI for accidental absolute importing of sibling subpackages?

@caiw caiw mentioned this issue Jan 26, 2024
23 tasks
@caiw
Copy link
Member Author

caiw commented Jan 26, 2024

Actually, option 2 editing $PYTHONPATH is made trickier by the fact it has to work inside the apptainer. @neukym do you know how to edit environmental variables (e.g. $PYTHONPATH) inside the container? I recall you saying just putting it in ~/.bash_profile didn't work (I tried that too).

@caiw caiw linked a pull request Jan 26, 2024 that will close this issue
@neukym
Copy link
Member

neukym commented Jan 26, 2024

Qustion: why doesn't this cause an issue with the CI tests?

Also - do we know why this was working for me, Tianyi and Ollie, but not @caiw?

@caiw
Copy link
Member Author

caiw commented Jan 26, 2024

Yes, I'm curious about that — did you add something like export PYTHONPATH=$PYTHONPATH":path/to/toolbox" into your .bash_profile, or its equivalent inside the container?

@caiw
Copy link
Member Author

caiw commented Jan 26, 2024

This was actually a problem with my poetry environment! When correctly set up, poetry should install an editable copy of the package in the pythonpath. Then the importing all works fine. See also #72.

@caiw caiw closed this as not planned Won't fix, can't repro, duplicate, stale Jan 26, 2024
@caiw caiw added the ❌ wontfix This will not be worked on label Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working 👷 design standards ❌ wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants