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

Restricting Python < 3.11 version and adding tensorflow, keras, scikeras, braindecode, skorch and torch [optional] #329

Merged
merged 17 commits into from Feb 13, 2023

Conversation

bruAristimunha
Copy link
Collaborator

Hi guys, @carraraig and @sylvchev!

I am updating the python version 3.8 >= to be unsure that we can add the new dependencies (TensorFlow, Keras, Scikeras, braindecode and Pytorch). In this pull, I added Tensorflow, Keras and Scikeras. In PR #328, I added the braindecode and torch.

I'm still learning to work with Poetry, but I think everything is correct. @sylvchev, can you take a look and apply the merge if that's good for you?

@bruAristimunha bruAristimunha added the dependencies Pull requests that update a dependency file label Feb 8, 2023
@bruAristimunha bruAristimunha added this to In progress in Benchmarking paper via automation Feb 8, 2023
@bruAristimunha bruAristimunha self-assigned this Feb 8, 2023
@bruAristimunha
Copy link
Collaborator Author

Hi @sylvchev,

Can I merge?

@sylvchev
Copy link
Member

sylvchev commented Feb 9, 2023

Thanks @bruAristimunha
The python 2.8 is already ensured in the pyproject.toml, is there a specific need for limiting version up to 2.11?
Regarding requirements of keras, tensorflow and scikeras, I'll prefer to add them as optional requirements. It is possible to do this in poetry (it is called dependency groups), like in pip, to specify requirements that are not installed by default but could be activated.
As these requirements add heavy implications on the system (GPU drivers, ...), it seems better to keep them separated to keep installation simple on most system.

@bruAristimunha bruAristimunha changed the title Updating Python version Restricting Python < 3.11 version and adding tensorflow, keras and scikeras [optional] Feb 9, 2023
@bruAristimunha
Copy link
Collaborator Author

bruAristimunha commented Feb 9, 2023

Hi @sylvchev,

About the limitation of python, scikeras requires this restriction. I think it's silly, but without that, poetry won't let you add the library, even optionally.

I defined the deep learning libraries as optional.

@bruAristimunha bruAristimunha changed the title Restricting Python < 3.11 version and adding tensorflow, keras and scikeras [optional] Restricting Python < 3.11 version and adding tensorflow, keras, scikeras, braindecode, skorch and torch [optional] Feb 10, 2023
@bruAristimunha
Copy link
Collaborator Author

I updated the command for the CI to install all dependencies

@bruAristimunha
Copy link
Collaborator Author

When I regenerated the poetry.lock, we lost something you made to solve a problem with docs. Do you know how to fix this, @sylvchev?

AttributeError: 'BuildEnvironment' object has no attribute '_write_doc_doctree_cache'

@bruAristimunha
Copy link
Collaborator Author

now, with this PR, we can follow and close the PRs #328 and #326.

Copy link
Member

@sylvchev sylvchev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in MOABB office hours, this is very nice.

@bruAristimunha bruAristimunha merged commit e2a32cd into NeuroTechX:develop Feb 13, 2023
Benchmarking paper automation moved this from In progress to Done Feb 13, 2023
@sylvchev
Copy link
Member

thanks @bruAristimunha

@bruAristimunha bruAristimunha mentioned this pull request Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants