-
Notifications
You must be signed in to change notification settings - Fork 124
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
build: moved to pyproject.toml/poetry #22
Conversation
Hi Erik. This great - thanks! Will do some review and testing very soon. In the meantime, a few other comments:
|
Yes, just tested (but with Edit: And reproduced the original issue in #19 (comment), looks like it's just a missing
Poetry will make that a lot easier for you :)
I'll take a look at it then, should be pretty easy. Edit: Done. |
661c32d
to
1aa3d08
Compare
@JadinTredup Should be ready for review now. The CI can be inspected at https://github.com/ErikBjare/eeg-notebooks/actions (it should work automatically in this repo too, once merged). |
I've extracted the CI stuff to a seperate PR #24, I'd appreciate a quick merge, so I can rebase this branch on top of upstream master. |
PR has been rebased on master following the merge of #24. Edit: Looks like the Windows build decided to start failing in this branch (see https://github.com/ErikBjare/eeg-notebooks/runs/1417206759?check_suite_focus=true) since it complains about missing Visual C++ build tools (which I thought should be included?). Weird. |
Trying to salvage this PR, but bumping into ridiculous dependency resolution times (caused by psychopy's ridiculous number of deps): So will take at least 50min. Hoping it finishes soon... I commented out psychopy to test, and it resolved in under 20min (iirc). |
lol, the OOM killer ended up killing it: dmesg: Not sure what to do about this, I guess ask in python-poetry/poetry#2094 or bring it up with the psychopy devs if they can loosen their dep requirements? (or whatever is taking forever) I guess I could try it again on my workstation with 64GB RAM + 64GB swap + 1Gbps? But would it even work? Edit: Running again now with |
Alright, I finally got a lock after ~1100 seconds on a different run, after I locked down some deps that I resolved by hand. |
Fixes #19, and more.
Changes
setup.py
/requirements.txt
replaced withpyproject.toml
/poetry.lock
poetry
pip install -r requirements.txt
is now simplypip install .
pip install .
will not use the exact versions inpoetry.lock
(but one can achieve the same thing by runningpoetry export -f requirements.txt; pip install -r requirements.txt
as usual). Version constraints specified in pyproject.toml will still be respected.Added basic GitHub Actions CI.(merged in ci: added basic ci #24)poetry
) and then runs: mypy for typechecking, pytest for testing (but there are currently no tests to run).DeprecationWarning
)pip install git+https://...
TODO (if requested)
Build docs in CI(merged in ci: added basic ci #24)requirements*.txt
files, include them inpyproject.toml
as dev dependencies or extras (that can be enabled with a flag)I haven't done any extensive testing, but all seems to work as expected.