search

askap-vast / vast-pipeline

This repository holds the code of the Radio Transient detection pipeline for the VAST project.
https://vast-survey.org/vast-pipeline/
MIT License
7 stars 3 forks source link

Change pip usage to poetry in test-suite #650

Closed ajstewart closed 2 years ago

ajstewart commented 2 years ago

Changes the use of pip in the test-suite to install the dependencies to poetry.

ajstewart commented 2 years ago

@marxide is this worth it?

If so I kind of feel like the user guidance to install using pip should also be removed and replaced with poetry, but I can't help but feel this is slightly less user friendly?

I tried to search about using pip for poetry managed projects and if it respects the lock file at all but I couldn't find much.

marxide commented 2 years ago

Maybe having the pipeline pip install-able is incompatible with wanting to use lock files. I couldn't find much on the topic either, but this SO answer recommends building wheels with poetry and then installing those with pip: https://stackoverflow.com/a/67534734. Our tests could do this automatically. For users, we could upload wheels when we make releases, which would maintain the current pip install compatibility. If users want to reliably build/install from source, they'll need Poetry.

Whatever we do, I agree that the tests should replicate what our docs tell users to do.

ajstewart commented 2 years ago

Yeah I'm not sure what the best method is here. Especially as it's not really a 'package' as such, it's purely only building dependencies, so setting it up for a pip install <package> doesn't seem that intuitive either. Is requiring users to use poetry a bad thing?

ajstewart commented 2 years ago

Decided not to pursue.