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

Should lesson code adhere to PEP8? #159

Open
petebachant opened this Issue Sep 10, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@petebachant
Contributor

petebachant commented Sep 10, 2015

Code like weight_lb = 2.2 * weight_kg should be weight_lb = 2.2*weight_kg according to PEP8. Does this matter here?

@gvwilson

This comment has been minimized.

Member

gvwilson commented Sep 10, 2015

@tbekolay

This comment has been minimized.

Contributor

tbekolay commented Sep 10, 2015

I'm all for PEP8 compliance (vigilant about it, even!) but I don't think your example follows PEP8. I'm assuming you're thinking about this section. Having no whitespace around * only applies when you have a longer expression with implicit order of operations (removing the space is to make the order of operations explicit, even though it doesn't make a different to the interpreter). I think in general PEP8 favours more whitespace than less whitespace; it also favours not rigidly adhering to PEP8 if it makes code less readable. 2.2*weight_kg is less readable to my eyes.

@petebachant

This comment has been minimized.

Contributor

petebachant commented Sep 10, 2015

Looks like I understood PEP8 wrong; sorry about that! My example does not fail the pep8 test script in either incarnation, though I usually prefer no whitespace around * to make it easier to distinguish addition from multiplication.

Maybe a compliance check could be automated by extracting all code from the lessons and running that through pep8? Is there a tool to convert Markdown to Python in this way, commenting out the text so line numbers are preserved?

@tbekolay

This comment has been minimized.

Contributor

tbekolay commented Sep 10, 2015

Such a check would be great, and could be incorporated into existing checks. I'm not aware of any tool out there to do this right now; if there were, it'd probably have to be customized anyhow since our code blocks are fenced off with ~~~ {.python}, which I've not seen elsewhere. Still, seems like not too tough of a script to write!

@petebachant

This comment has been minimized.

Contributor

petebachant commented Sep 10, 2015

Alright, I will take a crack at it when I get a chance.

@franktoffel

This comment has been minimized.

franktoffel commented Jan 28, 2016

@tbekolay @petebachant

This small script may help:
https://gist.github.com/Juanlu001/9082229

Are the python lessons in IPYNB format?

@gvwilson gvwilson self-assigned this Jul 31, 2016

@gvwilson gvwilson removed their assignment Sep 10, 2016

rgaiacs pushed a commit to rgaiacs/swc-python-novice-inflammation that referenced this issue May 6, 2017

tbekolay pushed a commit that referenced this issue Dec 19, 2017

Merge pull request #159 from rgaiacs/lc-svg-logo
Add Library Carpentry SVG logo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment