[WIP] Add 781 #922
Merged
[WIP] Add 781 #922
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Uses the configuration from ci_scripts
Based on the ci_scripts parameters.
Any venv folder does not need flake8. The example directory got flake8 warnings so I assumed it should be excluded.
Add ignore E203 as Black will observe PEPs specification for white space around a colon it is next to an expression.
There are a few places where big indentation is introduced that may warrant refactoring so it looks better. I did not refactor anything yet, but did exlude three (?) lists (of ids) to not be formatted.
This ensures it runs with the same versions and settings as developers.
Add two other developer dependencies not strictly required for unit tests, but required for development. I think the overlap between people who want to execute unit tests and perform commits is (close to) 100% anyway.
It seems to cause an error on import due to a missing sqlite3 dll. As we don't check coverage anyway, hopefully just uninstalling is sufficient.
|
I am getting some error from building docs/not finding a trace but I think they should be unrelated to this PR? Other than that I think it's good. |
* Sphinx issue fix * Removing comment
I ran into issues when the openml server config is not exactly 'https://www.openml.org/api/v1/xml', e.g. I had 'https://www.openml.org/api/v1'. I only noticed when getting a bad dataset url. This edit makes the API more robust against how exactly the server URL is set in the config.
This was fixed by @Neeratyoy in #923 |
mfeurer
reviewed
Jul 1, 2020
|
I'll update the PR tomorrow. If after that only small stuff remains, would you be able to handle the rest of the PR? I have vacation this week :/ (not great timing, but I figured the pre-commit will help us with the hackathon). |
Thanks a lot and sure, I can do that.
Enjoy :) |
Uses the configuration from ci_scripts
Based on the ci_scripts parameters.
Any venv folder does not need flake8. The example directory got flake8 warnings so I assumed it should be excluded.
Add ignore E203 as Black will observe PEPs specification for white space around a colon it is next to an expression.
There are a few places where big indentation is introduced that may warrant refactoring so it looks better. I did not refactor anything yet, but did exlude three (?) lists (of ids) to not be formatted.
This ensures it runs with the same versions and settings as developers.
Add two other developer dependencies not strictly required for unit tests, but required for development. I think the overlap between people who want to execute unit tests and perform commits is (close to) 100% anyway.
It seems to cause an error on import due to a missing sqlite3 dll. As we don't check coverage anyway, hopefully just uninstalling is sufficient.
|
I think this should have done the trick 👍 (rebase and processed feedback) |
mfeurer
approved these changes
Jul 2, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a pre-commit hook with flake8, mypy and black.
This is the first time black is used in the project, so this PR also reformats the code with black.
The noticeable files are:
.flake8: a flake8 configuration.
Used by pre-commit but convenient for users too.
Added ignore E203, which normally prohibits whitespace around the colon.
Black inserts white-space around the colon sometimes as per PEP (Whitespace in expressions and statements).
I could not find where
exampleswhere before explicitly ignored for flake8, but the develop branch examples raised problems with flake8, so I assumed we are excluding this folder.mypy.ini: a mypy configuration. Used by pre-commit but convenient for users too.
.pre-commit-config.yaml: the pre-commit configuration
I fixed the versions so everyone works with the same checkers. These are the versions used by GAMA, but not chosen for any particular reason (they were most recent at the time, I believe).
contribution guidelines in docs