search

Bioprotocols / container-ontology

Container ontology for use with PAML (Protocol Activity Markup Language)
Apache License 2.0
3 stars 3 forks source link

[bug] Python API Install Instructions #22

Closed tsdobbs closed 2 years ago

tsdobbs commented 2 years ago

The README says the following:

To install this library into python (we recommend using a dedicated virtual environment), assuming that your current working directory is the directory of the container-ontology git repo, you may simply do: pip install -e .

However, while there is a setup.py in the main directory, and it installs a module called owlery_client, the module that got installed for me was empty. I was able to get the correct behavior by navigating to the owlery_client subfolder cd owlery_client, then running pip install -e .

Can this issue be reproduced? If so, either the file structure should be changed to match the README, or the README should be changed to match the observed behavior.

rpgoldman commented 2 years ago

Thanks, Tim. This is a packaging issue with the auto-generated owlery_client library. I’m going to remove it and make it into a dependency.

rpgoldman commented 2 years ago

I think I have fixed this. If you checkout the iss22 branch (from the Pull Request), and retry the pip installation, it should work now.

If that does not fix your problem, please LMK and put here any error messages.

If this does work for you, I will merge that branch in.

tsdobbs commented 2 years ago

It does work for me now, although when I run python -m container_api.client_api from the src folder, I get the following error:

 RuntimeWarning: 'container_api.client_api' found in sys.modules after import of package 'container_api', but prior to execution of 'container_api.client_api'; this may result in unpredictable behaviour
  warn(RuntimeWarning(msg))

Looks to me like an issue with loading the modules in order

rpgoldman commented 2 years ago

This is an oddity of running the module after loading the module.

It may be a problem with the way I put the main() procedure in that file. When we load it in this way, it loads the whole container_api, and then it loads itself, which seems to confuse the interpreter.

It does seem to work fine despite that warning, but I would love to figure out how to make it go away.

rpgoldman commented 2 years ago

A little research shows that I should pull the main() procedure out of client_api and put it in a __main__.py file, which you would run as python -m container_api. See StackOverflow

I will make that change.

tsdobbs commented 2 years ago

This seems to have solved that problem, but now I see the error ModuleNotFoundError: No module named 'container_api.api_client'

Inspecting the package, which seems to have installed correctly, I see that the submodule is client_api, not api_client. Maybe a typo?

rpgoldman commented 2 years ago

I think this should be fixed now. Please see the new branch. note that now you should do:

 python -m container_api

I will try to figure out how to get testing set up on this repository, but doing that requires starting the container server and then talking to it, and I don't know how to do that in GitHub actions.

tsdobbs commented 2 years ago

After the latest push, following the instructions in the current README to run python -m container_api.client_api, I get a similar error about import order:

/usr/lib/python3.9/runpy.py:127: RuntimeWarning: 'container_api.client_api' found in sys.modules after import of package 'container_api', but prior to execution of 'container_api.client_api'; this may result in unpredictable behaviour
  warn(RuntimeWarning(msg))

Also, this is the only output I get - none of the feedback from the server about containers.

rpgoldman commented 2 years ago

Are you running from the (still-unmerged) branch?

Also, if you look at the README for the branch, here, the instructions have changed to:

python -m container_api

I think you have been running the latest code, but with the old instructions, and that explains why you get no output: the main routine has been moved to __main__.py and is no longer in client_api.py

tsdobbs commented 2 years ago

Thanks @rpgoldman. Yes - it looks like my previous comment was based on me erroneously using the main branch README instead of this branch's README. Everything seems to be working now! Here's the output I got, which looks like the intended output:

List of matching instances is:
        https://sift.net/container-ontology/strateos-catalog#Corning96-ubottom-clear-tc
        https://sift.net/container-ontology/bbn-plate-catalog#NEST_0.1_mL_96-Well_PCR_Plate_Full_Skirt
Checking matching plates that are also available at Strateos.
List of matching instances AT STRATEOS is:
        https://sift.net/container-ontology/strateos-catalog#Corning96-ubottom-clear-tc short name: 96-ubottom-clear-tc
rpgoldman commented 2 years ago

Closed with #23