njcuk9999 / lbl

Line by line code for radial velocity
MIT License
16 stars 3 forks source link

pip install missing resources package #35

Open j-faria opened 1 year ago

j-faria commented 1 year ago

The lbl/resources folder doesn't get included in a pip install, resulting in

ModuleNotFoundError: No module named 'lbl.resources'

This can be easily fixed by adding a (empty) __init__.py file in that folder.

njcuk9999 commented 1 year ago

Done.

I've never got this ModuleNotFoundError so I guess you aren't using: pip install -U -e . as per the readme instructions, is that correct? My guess is that when the you install via pip and keep the git repo editable this doesn't matter (as pip doesn't move the directory to the site-packages)

j-faria commented 1 year ago

Thank you! 👍🏼

My guess is that when the you install via pip and keep the git repo editable this doesn't matter (as pip doesn't move the directory to the site-packages)

Exactly, with the -e it still works because it picks up the "local" directory. I was using

pip install https://github.com/njcuk9999/lbl/archive/main.zip 
j-faria commented 1 year ago

Oops, closed the issue too soon. The data_str_readme.md file still doesn't get moved to site-packages which makes the copy_readme function fail.

Maybe it would be easier to hardcode the "data structure read me" in lbl_misc.py?

njcuk9999 commented 1 year ago

I don't want to hard code this it isn't the point. There should be a way to write a manifest of files that are copied over.

Just out of curiosity is there a reason you can't use pip install -U -e . for now?

j-faria commented 1 year ago

I don't want to hard code this it isn't the point. There should be a way to write a manifest of files that are copied over.

Agree! I think it might be working with the new manifest file.

Just out of curiosity is there a reason you can't use pip install -U -e . for now?

No strong reason, but I'm using lbl within a CI / action and wanted to specify it as a dependence instead of git cloneing and pip installing locally. More generally, requiring a user to clone the repository and have a local copy may not be feasible in some cases (although I'm very much guilty of that myself!). Since it's relatively easy to pip install from the repo directly, I thought it would be nice to have the option.

njcuk9999 commented 1 year ago

Yer it was the plan eventually to use pip install only but we are still in development and not having the git easily updatable is a pain (especially when we don't change the version for every small change, which means pip wont update lbl).

The MANIFEST.in should work now though.