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

Updated VectorBender Coding Style #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

busterbeam
Copy link

@busterbeam busterbeam commented Jul 20, 2021

As per your request this Push Request has been been completely rewritten so as to focus as closely as possible to just code styling. I preformed a pass with black before and after editing the scripts. If this looks good I'm itching to make the logic flow better, also line 46 of vectorblenderhelp.py seems to have an error, changing from self to cls object instance keywords, wondering if this might be the reason for crashes?


Mainly focused on code styling, using black style guide. Changed from old format % specifiers to new f-literal strings. Removed u prefixing on strings as all strings are Unicode in python unless otherwise specified. Any further changes where purely cosmetic no conditional logic, variable, method or class names were changed. Mild readability improvements

Mainly focused on code styling, using black style guide.  Changed from old format `%` specifiers to new f-literal strings.  Removed `u` prefixing on strings as all strings are Unicode in python unless otherwise specified.  Any further changes where purely cosmetic no conditional logic, variable, method or class names were changed.  Mild readability improvements
@busterbeam busterbeam changed the title Add files via upload Updated VectorBender Coding Style Jul 20, 2021
@olivierdalang
Copy link
Owner

Thanks for that, it looks good.

Could you also add a pre-commit setup to automatize black formatting (see https://pre-commit.com/), and add a line in the readme to explain required steps before contributing (pip install pre-commit then pre-commit install) ?

About line 46 no cls is just a regular variable (with indeed a confusing name). It does not by itself cause any issue.

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