Refactor watermark #65
Conversation
* sort imported modules * use dedent to indent multi-line string
* get information from each function separately * Combine different information in one function instead of all over the object * Sort imports and other part
| @@ -0,0 +1,2 @@ | |||
| pycodestyle: | |||
| max-line-length: 88 # To be compatible with black | |||
rasbt
Oct 26, 2020
Owner
curious: why particularly 88, and what's black?
curious: why particularly 88, and what's black?
GreatBahram
Oct 27, 2020
Author
Contributor
Well, generally the community answer to make the PEP8 code style was to use a lint tool like flake8 or pycodestyle and etc. These tools only report the problem, do not fix them. On the other hand, black is an auto-code-formatter (as they say).
You just run it and it automatically fixes the problem. There are also some Jupyter extensions, such as this one, that run black inside Jupyter notebook.
And by the way, black uses 88, by default, characters per line.
You can read more about it on their documentation site. It's a great tool and I hope you’ll check it out!
Well, generally the community answer to make the PEP8 code style was to use a lint tool like flake8 or pycodestyle and etc. These tools only report the problem, do not fix them. On the other hand, black is an auto-code-formatter (as they say).
You just run it and it automatically fixes the problem. There are also some Jupyter extensions, such as this one, that run black inside Jupyter notebook.
And by the way, black uses 88, by default, characters per line.
You can read more about it on their documentation site. It's a great tool and I hope you’ll check it out!
jamesmyatt
Oct 27, 2020
•
Contributor
black is awesome. Look at https://www.thoughtworks.com/radar/techniques/opinionated-and-automated-code-formatting and https://www.youtube.com/watch?v=esZLCuWs_2Y
Also isort
black is awesome. Look at https://www.thoughtworks.com/radar/techniques/opinionated-and-automated-code-formatting and https://www.youtube.com/watch?v=esZLCuWs_2Y
Also isort
|
Hello @GreatBahram! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! Comment last updated at 2020-11-19 16:01:15 UTC |
|
Nice work, I agree that the restructuring (and use of lists) will make everything a bit more manageable. |
|
If the official code style is now "black", can you update the Readme to include this? e.g. there's a developer guidelines section now. |
|
Thanks for the clarification @GreatBahram . The tool (black) looks really nice. I would prefer to stick with the PEP8 80-char though. Personally, I also have a slight preference for the "# Aligned with opening delimiter" style in PEP8. I.e., I find
more readable than
Both are good styles, and I also sometimes use the latter, but in this case, I really think the upper one is a tad more readable. No worries, I can make these little changes (just personal preferences) to this otherwise awesome PR. |
|
Just going through code again, I really like this refactoring. Also, thanks for improving the output style (capitalization etc.)! Awesome PR! |
|
Thank you so much for your compliment! |
Hi there, @rasbt
I refactored the watermark module:
watermarkmethod as it was._generate_formatted_text, which is fine in my opinion, even if someone wants to elaborate it to produce much fancy string it is easier than before.PS: I added a
.pep8speaksfile to increase the line length to 88. I hope you find this pull request useful.Thanks