kammerje / spaceKLIP

Pipeline for reducing JWST high-contrast imaging data. Published in Kammerer et al. 2022 and Carter et al. 2022.
https://ui.adsabs.harvard.edu/abs/2022SPIE12180E..3NK/abstract
MIT License
16 stars 9 forks source link

requirements.txt always grabs from github #143

Closed JarronL closed 2 weeks ago

JarronL commented 6 months ago

In the requirements.txt document, we currently have these calls:

webbpsf_ext @ git+https://github.com/JarronL/webbpsf_ext.git@develop
pyklip @ git+https://bitbucket.org/pyKLIP/pyklip.git@jwst_new

This means if I do something like pip install -e . it appears to always grabs these branches from github even if I already have them installed, or if I'm working on a different dev branch in webbpsf_ext, for instance. Instead, I would like to install with something like -r requirements-dev.txt but I see that is currently for readthedocs generation.

My recommendation::

  1. Copy appropriate contents of requirements.txt and requirements-dev.txt to a new file at docs/requirements-docs.txt
  2. In readthedocs.yml change requirements to only use docs/requirements-docs.txt
  3. In current requirements-dev.txt replace with contents of requirements.txt but remove reference to @ git+[...]

So, in the end::

Thoughts?

mperrin commented 6 months ago

I believe the main issue which motivated this was that right now, even “production” non-development use needs that special branch of pyKLIP, and there’s no way to install that except direct from GitHub. I have repeatedly encouraged trying to make progress getting that branch merged but it hasn’t risen to the top of anybody’s to do list yet. I’d really really hope we can do that soon.

For webbpsf_ext I think your proposed approach is fine and good, I just don’t think it (yet) suffices for pyklip. And yeah we can probably separate dev from docs like you say.

AarynnCarter commented 6 months ago

@mperrin I think https://github.com/kammerje/spaceKLIP/pull/139 is the last thing that needs reviewing before we can make the V2.0 release, and subsequently start working on merging the pyklip branches.

My time is a little tight until I start in a week or so, but I'm happy to hand to off merging that PR (it's relatively small) and formally releasing the new main branch PR to you or @kammerje if we want to move a little quicker.

JarronL commented 6 months ago

For webbpsf_ext I think your proposed approach is fine and good, I just don’t think it (yet) suffices for pyklip. And yeah we can probably separate dev from docs like you say.

I think this approach still works for pyklip in the narrow case that I'm discussing. Most of the developers here are aware of the necessary branch changes and are well equipped to do that manually as needed. When I'm performing development on other spaceKLIP branches, I occasionally perform pip install commands to make sure everything works, so pip install -r requirements-dev.txt -e . without invoking a github download would be what I want.

Or I could just make my own requirements-jml.txt.

kammerje commented 2 months ago

@JarronL I see that the installation of WebbPSF_ext has already been updated to not install from GitHub anymore. The same should soon be possible for pyKLIP. For now, I think it is best to pull the installation of pyKLIP from Bitbucket out of the requirements file and do it separately (in fact, I had to do this to install git-lfs via Anaconda before which now seems to be required to pull pyKLIP from Bitbucket), see installation instructions on GitHub here.

AarynnCarter commented 3 weeks ago

@JarronL Do you think your suggestion would still be valuable now we aren't doing the github calls? Reading things over I think maybe what we have will still work well.

AarynnCarter commented 2 weeks ago

Closing after conversation with Jarron