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

Add pyproject.toml to config black line length #258

Merged
merged 4 commits into from
Nov 7, 2019
Merged

Add pyproject.toml to config black line length #258

merged 4 commits into from
Nov 7, 2019

Conversation

janosh
Copy link
Member

@janosh janosh commented Oct 23, 2019

@janosh
Copy link
Member Author

janosh commented Oct 23, 2019

@ardunn Like you said, there seems to be a problem with code style. But I'm not sure it's the code. The dev_scripts/run_code_style_check.sh script doesn't log any output, suggesting that maybe it's not running as expected?

#!/bin/bash -eo pipefail
source dev_scripts/setup_env_dev.sh
source dev_scripts/run_code_style_check.sh
WARNING: You are using pip version 19.3; however, version 19.3.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Exited with code 1

Similarly, the tests fail with something that seems unrelated to the changes in this PR.

RuntimeError: implement_array_function method already has a docstring
2019-10-23 09:43:30,877 - ERROR - environment variable CODACY_PROJECT_TOKEN is not defined.
Exited with code 1

@ardunn
Copy link
Contributor

ardunn commented Oct 23, 2019

Coce style

Ok so @janosh I ssh'd into your build and ran the script manually:

(test_env) circleci@5995ac2386af:~/py372_automatminer$ source dev_scripts/run_code_style_check.sh
Code misformatted!
automatminer/presets.py:143:86: E501 line too long (88 > 85 characters)
automatminer/tests/test_pipeline.py:79:86: E501 line too long (86 > 85 characters)
automatminer/featurization/tests/test_sets.py:74:86: E501 line too long (88 > 85 characters)
automatminer/preprocessing/feature_selection.py:223:86: E501 line too long (86 > 85 characters)
automatminer/automl/adaptors.py:204:86: E501 line too long (87 > 85 characters)

I just don't think CircleCI prints the output sometimes if a command exits with code 1.

Tests failing

The tests actually passed. The environment variable thing is another CircleCI problem (I posted a question on the forum about it). Don't worry about it, when the code_style stuff is fixed I'll just merge it and trigger the tests myself (which would fix this env var issue until CircleCI gets back to me) :)

Edit: Note to self- https://circleci.com/blog/managing-secrets-when-you-have-pull-requests-from-outside-contributors/

@janosh
Copy link
Member Author

janosh commented Oct 23, 2019

Sorry, this is my mistake. Looks like we need a pyproject.toml file after all since that's the only config file black supports. Should have checked that as part of #245.

@ardunn
Copy link
Contributor

ardunn commented Oct 25, 2019

@janosh would it be possible to revert the formatting on these files and just PR the config changes? I can reformat on my own but it is hard to review 55 files before merging to the master branch

Again thanks so much for the PR!!!

@janosh janosh changed the title Auto-format all .py files Add pyproject.toml to config black line length Oct 25, 2019
@janosh
Copy link
Member Author

janosh commented Oct 25, 2019

would it be possible to revert the formatting on these files and just PR the config changes?

@ardunn Done.

@janosh
Copy link
Member Author

janosh commented Nov 6, 2019

@ardunn Anything left to do here?

@ardunn ardunn merged commit 8297032 into hackingmaterials:master Nov 7, 2019
@ardunn
Copy link
Contributor

ardunn commented Nov 7, 2019

@janosh thanks again!

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.

2 participants