vallis / libstempo

libstempo — a Python wrapper for tempo2
MIT License
29 stars 34 forks source link

Make new release on PyPI #28

Closed mattpitkin closed 3 years ago

mattpitkin commented 3 years ago

Could a new PyPI release of libstempo be made, so that Enterprise can be installed from a versioned version of libstempo rather than having to install from the git repo?

jellis18 commented 3 years ago

@mattpitkin I'm working on this now and will be doing it all through github actions triggered through a release. I may also need your help as it would be great to have a conda release through actions as well so it is all packaged together in one place

mattpitkin commented 3 years ago

@jellis18 Cool. I've been playing around with GitHub actions for package CI testing and PyPI releases myself in the last week too! For the conda release, it should get automatically triggered by the upload of any new version to PyPI. However, due to the way the releases are done on conda-forge I think there'll always need to be a manual merge of the generated PR.

jellis18 commented 3 years ago

I'm testing this stuff out now in my fork here. I'll make a PR to the main Repo soon.

In terms of conda, whatever is done it should be triggered with this release, as you said. I'm not a huge fan of conda and never use it but I know NANOGrav uses it a lot.

Also, this is basically a weekend project for me so I may not get too much done during the week.

mattpitkin commented 3 years ago

I'm testing this stuff out now in my fork here. I'll make a PR to the main Repo soon.

Looks good. In case it's any use (although you've probably seen examples elsewhere) this is my yml for uploading a package to PyPI https://github.com/mattpitkin/lintegrate/blob/master/.github/workflows/pypi.yml - it uploads a source distribution (there's not a valid wheel for that package) when a new release tag is added on Github. Do you have admin access to the PyPI release for libstempo or will @vallis need to add the secret token?

In terms of conda, whatever is done it should be triggered with this release, as you said. I'm not a huge fan of conda and never use it but I know NANOGrav uses it a lot.

This automated bot trawls new PyPI releases and will generate a conda-forge feedstock PR for libstempo here - as I'm the maintainer of that I'll get a notification and can very easily and quickly (depending on the time of arrival of the notification!) review and merge the request to generate the release. If you want I can add you as a maintainer so that you have the ability to merge things?

Also, this is basically a weekend project for me so I may not get too much done during the week.

No problem. I'm in no major rush for the new release to be made.

vallis commented 3 years ago

Thank you so much Justin and Matt!

I understand the rationale behind splitting off install_tempo2.sh, but then "pip install libstempo" won't be as useful because the user will need to download the libstempo source manually to get install_tempo2.sh, correct? However the user could do "conda install tempo2; pip install libstempo" or "conda install tempo2; conda install libstempo".

Another question is to what extent we need to harmonize the requirements.txt for libstempo and enterprise. I suppose we'll be fine as long as all the enterprise versions are higher, but in some cases the libstempo versions are pinned. What happens then?

jellis18 commented 3 years ago

I understand the rationale behind splitting off install_tempo2.sh, but then "pip install libstempo" won't be as useful because the user will need to download the libstempo source manually to get install_tempo2.sh, correct? However the user could do "conda install tempo2; pip install libstempo" or "conda install tempo2; conda install libstempo".

That is a great point that I completely missed. In my experience its good practice to have system wide dependencies specified beforehand and not have the python package try to guess. But as you say, that script is basically useless to a user installing via pip.

My overall idea is like you mentioned at the end and I would update this in the README.

Install via conda

I think the libstempo conda package can have the conda tempo2 as a requirement

conda install libstempo

As I mentioned above I'm not too keen on conda. As long as there was a good way to tie tempo2 tags to conda packages it may be ok...

Install via pip

First install tempo2

# on mac
brew install tempo2

# on linux
apt install tempo2

Note, I'm planning on making these

pip install libstempo

Another question is to what extent we need to harmonize the requirements.txt for libstempo and enterprise. I suppose we'll be fine as long as all the enterprise versions are higher, but in some cases the libstempo versions are pinned. What happens then?

I think the requirements are pretty flexible in my branch (which I'll make a PR in the next few days)

vallis commented 3 years ago

It would be great to have tempo2 on homebrew and apt-get... since those would be able to install the tempo2 requirements (gsl, blas, calceph) as conda does. On the other hand, they will be more things to keep synchronized unless those packages always build from the latest source.

If "pip install libstempo" does not find tempo2, it can then suggest using brew or apt. If could also propose a one-liner that would download install-tempo2.sh from github and run it (which would require gsl, etc. to have been installed already).

jellis18 commented 3 years ago

@vallis we could propose this for a one liner curl -sSL https://raw.githubusercontent.com/vallis/libstempo/master/install_tempo2.sh | sh

I'm going to make an MR to your fork today.

jellis18 commented 3 years ago

My PR related to this is here

vallis commented 3 years ago

The PR is now merged, thanks! I reopened the discussion on conda packaging in #34.