Skip to content
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

fix: support Python 3.8 (and 3.9 on Linux) #32

Merged
merged 6 commits into from
Mar 26, 2021

Conversation

ErikBjare
Copy link
Collaborator

@ErikBjare ErikBjare commented Dec 2, 2020

Depends on #31

Fixes #29, Fixes #50

Based on a similar solution in: ActivityWatch/aw-watcher-afk#44

@ErikBjare ErikBjare changed the title fix: fixed pywinhook to only be a requirement on Windows fix: use prebuild wheels for pywinhook for easier Windows installation Dec 2, 2020
@ErikBjare ErikBjare changed the title fix: use prebuild wheels for pywinhook for easier Windows installation fix: use prebuilt pywinhook wheels for easier Windows installation Dec 2, 2020
@ErikBjare
Copy link
Collaborator Author

Looks like requirements.txt doesn't accept user-supplied wheels for dependencies with pypi entries.

Another reason to look into #22 I guess.

@ErikBjare
Copy link
Collaborator Author

ErikBjare commented Dec 2, 2020

Might just have been the fact that wheels for Python 3.6 weren't available, bumping CI Python version to check if that's the case.

@ErikBjare
Copy link
Collaborator Author

ErikBjare commented Dec 2, 2020

CI passed, ready to merge (after #31) 🎉

@ErikBjare ErikBjare changed the base branch from master to cli December 2, 2020 14:33
@ErikBjare ErikBjare changed the base branch from cli to master December 2, 2020 14:33
@ErikBjare ErikBjare mentioned this pull request Dec 3, 2020
@JohnGriffiths
Copy link
Collaborator

Just want to check: do we think the user-supplied wheels is the best option here, or is the pywinhook issue largely solved by moving to python 3.7?

@ErikBjare
Copy link
Collaborator Author

ErikBjare commented Dec 4, 2020

@JohnGriffiths From a second look at https://pypi.org/project/pyWinhook/#files it seems like there exists pywinhook wheels on PyPI for Python 3.7 and 3.8 (news to me), but indeed not for 3.9.

Since there are wheels available for 3.9 (here: https://www.lfd.uci.edu/~gohlke/pythonlibs/), we might want to change this PR to only supply prebuilt wheels for Python 3.9 (and maybe for 3.6 since that's not on PyPI either, but we'll probably want to drop 3.6 support soon anyway).

Edit: It should be added that I intend to resolve issues with newer Python versions, as I've laid out in #50

@ErikBjare ErikBjare changed the title fix: use prebuilt pywinhook wheels for easier Windows installation fix: support Python 3.9 Dec 4, 2020
@ErikBjare
Copy link
Collaborator Author

Supports Python 3.9 on Linux now, but one remaining issue with tables 3.6.1 on macOS and Windows where wheels are missing and requires HDF5 headers to be available (see #50)

@ErikBjare ErikBjare changed the title fix: support Python 3.9 fix: support Python 3.8 (and 3.9 on Linux) Dec 4, 2020
@ErikBjare
Copy link
Collaborator Author

Looks like pyriemann broke with the latest scikit-learn, submitted a fix: pyRiemann/pyRiemann#93

@JohnGriffiths
Copy link
Collaborator

I'm OK with merging this.

But shlukd we be e currently waiting for the pyriemann fix PRs to be accepted before doing so?

@ErikBjare
Copy link
Collaborator Author

If so, we'll need to lower the sklearn constraint again, and leave Python 3.9 support for another PR.

I'd suggest we just wait for it to get merged (or if it takes too long, fork pyriemann ourselves and switch to using that).

@ErikBjare ErikBjare merged commit 00b1817 into NeuroTechX:master Mar 26, 2021
@ErikBjare ErikBjare deleted the dev/pywinhook-wheels branch March 26, 2021 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support newer Python versions PyWinhook building wheel error
2 participants