Setup - Fix pip installation #27

Closed
tfardet wants to merge 1 commits from pip-fix into main
Owner

This PR fixes #26 and should hopefully enable automatic requirements installation with pip.

This PR fixes #26 and should hopefully enable automatic requirements installation with pip.
tfardet added the
bugfix
installation
labels 10 months ago
tfardet added 1 commit 10 months ago
Collaborator

Hi, today I had some time and I think I found out what was happening with the dependencies. Nothing to do with the test pypi server, BTW.

Relevant SO thread:
https://stackoverflow.com/questions/2058802/how-can-i-get-the-version-defined-in-setup-py-setuptools-in-my-package

All the answers there are worth a read and quite instructive, but this one is exactly what has happening to us ("it will work for you")

Warning about race condition during install

By the way, DO NOT import your package from your setup.py as suggested in another answer here: it will seem to work for you (because you already have your package's dependencies installed), but it will wreak havoc upon new users of your package, as they will not be able to install your package without manually installing the dependencies first.

I have implemented the solution provided in the official docs here:
https://packaging.python.org/guides/single-sourcing-package-version/#single-sourcing-the-package-version

in this WIP commit 6b0b3d2f0f:

It works in a fresh conda environment, installing mpl-chord-diagram neatly. It already contains c86251c75a 'Setup - Fix pip installation'. Haven't done a PR because IDK how you want't to go about this, plus I have some other minor suggestions (some of them are as comments in the commit).

In particular, moving files somewhere and then moving them back using a try/except/pass construct is dangerous. I issue
python setup.py develop
then files disappear from the root dir, but never get moved back so the root directory is essentially broken and has to be restored by hand (Took me a while to figure out that some files had been moved). Again, this construct might have to do with the requirements for the git submodule to work, and this is just my two cents, BUT it seems dangerous.

What are your thoughts?

Hi, today I had some time and I think I found out what was happening with the dependencies. Nothing to do with the test pypi server, BTW. Relevant SO thread: https://stackoverflow.com/questions/2058802/how-can-i-get-the-version-defined-in-setup-py-setuptools-in-my-package All the answers there are worth a read and quite instructive, but this one is exactly what has happening to us ("it will work for you") > # Warning about race condition during install > > By the way, DO NOT import your package from your setup.py as suggested in another answer here: it will seem to work for you (because you already have your package's dependencies installed), but it will wreak havoc upon new users of your package, as they will not be able to install your package without manually installing the dependencies first. I have implemented the solution provided in the official docs here: https://packaging.python.org/guides/single-sourcing-package-version/#single-sourcing-the-package-version in this WIP commit https://codeberg.org/gph82/mpl_chord_diagram/commit/6b0b3d2f0fda51f8328e9ed4ab7242dc0e0096b7: It works in a fresh conda environment, installing mpl-chord-diagram neatly. It already contains c86251c75a3580ef0e25e39aa676c6527196b6fd 'Setup - Fix pip installation'. Haven't done a PR because IDK how you want't to go about this, plus I have some other minor suggestions (some of them are as comments in the commit). In particular, moving files somewhere and then moving them back using a try/except/pass construct is dangerous. I issue `python setup.py develop` then files disappear from the root dir, but never get moved back so the root directory is essentially broken and has to be restored by hand (Took me a while to figure out that some files had been moved). Again, this construct might have to do with the requirements for the git submodule to work, and this is just my two cents, BUT it seems dangerous. What are your thoughts?
Poster
Owner

Hi there, sorry for the long delay...
I'm fine with your implementation so feel free to apply all changes you suggested in the comments and make a new PR 👍

Hi there, sorry for the long delay... I'm fine with your implementation so feel free to apply all changes you suggested in the comments and make a new PR :+1:
Collaborator

cool

cool
tfardet closed this pull request 10 months ago
Poster
Owner

Superseeded by #28

Superseeded by #28
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Loading…
There is no content yet.