-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Swap Packaging to Poetry #60
Conversation
- main thing is removal of setup.py and requirements.txt, instead we have pyproject.toml and poetry.lock. pyproject is the project description that contains everything that setup.py does (i was careful to keep it as similar as possible to the existing one while also adding version constraints that allow a successful build/install), and the poetry.lock contains the exact specification of what to install - I bumped up the minimum python from 3.5 to 3.6.1 to be able to install a few packages, both 3.5 and 3.6 are EOL and we should think of bumping this up further. Also - added some type hints to the DLCLive class which was bothering me to not have tab completion when i've been using it lol - DLCLive object tries to mutate something that is by default a tuple, which should cause an error if anyone tries to use tflite and dynamic (instead of warning and repairing as is intended) - Using Pathlib to resolve the path and check that it exists when loading configuration, this gives more helpful error messages bc people know where DLC is looking in their filesystem/resolves ambiguity of relative paths - added a CITATION.cff file <3
- removed pip install requirements - removed pytest tests (i don't see any!?) - installing package and running test from already checked-out copy rather than pip installing from git:// - the packaging script doesn't look like it's doing anything, but i'd be happy to write one that autodeploys on tagged commits to master
…d then call it when doing the tests - also install and run using poetry because windows paths are an abominable hell - tensorflow 2.7 only support python >3.7
…e because it gets instantiated before the check happens whether it should be saved or not. also limited tf versions for python <3.7 because it has to like do the ridiculous pip thing where it downloads all of them for some reason
uh so it turns out that the test function downloads the video but then it just streams the raw video from github anyway because the video file string that's passed is just the url DeepLabCut-live/dlclive/check_install/check_install.py Lines 25 to 28 in 36e9d08
|
- also cleaned up file handling in test function. Previously the video file was downloaded and not used, and lots of implicit paths that seem to be causing people trouble when running it.
OK damn that was harder than i thought it was gonna be, but packaging should be a bit neater now. supporting 3.6 was causing a huge amount of problems and making pip spend hours trying to install all the possible versions of things, it's EOL anyway so I dropped it, but we can make it technically allowed if ya want, tho the tests need some more work to run :) also fixes test errors in 36e9d08 from trying to open tk without xvfb. The two gh actions seem to overlap, but keepin em in there for now. |
Hey! question, as I am new to Poetry (looks awesome); how can I use |
that numpy ~1.18 sticks out like a sore thumb and really pins a lot of other packages down/complicates build, i tried to keep it bc it had been mentioned in prior issues and etc, but any reason why we shouldn't try and catch up? |
sry didn't refresh. is that script for like a development environment? like keeping the running version in sync as you change code? the generic way is to that's a replacement for a the more explicit dependency spec makes it much easier for pip and poetry to verify the environment is as it should be, so repeatedly doing "poetry install" also works without much overhead lol. or lmk if I've missed the purpose of that script ❤️ edit: their docs are actually really good imo and basically always have them open when I'm mashing at my versions https://python-poetry.org/docs/cli/ |
Hey @sneakers-the-rat just so I undetstand, poetry would replace pypi packaging fully then (i.e., I wouldn't push to pypi packages such that |
it would replace setuptools packaging, but you would still be able to publish to pypi : |
re: #59
Some light maintenance, updating from the old setuptools format to new
pyproject.toml
format using Poetry.3.6.1edit:3.7
to be able to install a few packages, both 3.5 and 3.6 are EOL and we should probably deprecate3.tegra
inplatform()
, because I wasn't sure exactly where in the platform string it shows up andplatform()
is sorta a mash of several other environment markers, but if we know that it's relatively easy to spec with Poetry."^2.7.0"
) but that can be relaxed/tightened/etc. as needed.Also
Runs fine in a venv,
python==3.7.12
Packages: