-
Notifications
You must be signed in to change notification settings - Fork 3
Improving the pre-commit and making sure that everybody can install eegdash #27
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
Conversation
All good on the features module. |
Thanks @bruAristimunha . Why was the main function removed from main.py? |
Can you please review @arnodelorme, @avivdotan and @dungscout96? We need more tests. |
Thanks Bruno. What was the main change that fix the installation issue? I see addition of test files and a lot of reformatting which seems like automatic fixes of the pre-commit hooks which is great. But not sure what the main functional change is. And is the change from main.py to api.py a standard practice? |
Yes, It's better if we don't put main.py in python code pattern perspective |
As Arnaud is starting a second PR based on this one, I understand that we should apply the merge here. |
At the moment, there were some problems instantiating datasets that were not being captured in CI. I'm adjusting them now and creating a test. In addition, there were several codes that made no sense in the code, comments from old code, which goes against the rules of good versioning.