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

feat(lesson): pep8, linters & code formatters #12

Merged
merged 19 commits into from
Oct 17, 2024

Conversation

lwasser
Copy link
Member

@lwasser lwasser commented Oct 7, 2024

todo: add to bottom of intro clean code page:

[^zenofpython]: Paste this into your shell: python -c 'import this'``

> Local application/library specific imports.
> You should put a blank line between each group of imports.

You may be wondering, what is a standard library import? The standard library imports are commonly used tools that are general in purpose and are part of the standard library of **Python**. These including things like:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"that are included with python," "that you don't have to install," with link to https://docs.python.org/3/library/index.html ?


import matplotlib.pyplot as plt
import numpy as np
import pandas as pd
Copy link
Contributor

@sneakers-the-rat sneakers-the-rat Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add the third block of

# imports from your local package
import mypackage
from mypackage.internal import _secret_function

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i see you have a separate section for that. my b. i just figured showing all three of them in the example that follows the list of 3 would be nice, but this is a draft and i'm jumping the gun on giving comments lol

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go for it!! comment away. i just combined two pages. i'm totally fine with consolidating!!

This is good, too, because in our second workshop, they will create a package and import it. so they will get to try this out!!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just pushed a new version of this and made all of the fixes that you suggested. I also redesigned the structure to be:

  • Example of good vs not as good code format.
  • How to apply tools to help you make your code prettier :)
  • Why use these tools and standards
  • Some rules to remember

That way, we start with something that doesn't have to be so hard and end with the details that many people will skip (I think?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hear you on that but i think that currently it's a little bewildering the other way where we're missing some introductory information on what we're talking about and why first, about to post the review

edit: oh wait it makes this comment part of the review even when we started it before that lol my b

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆 no worries!! i think / hope that i addressed your comment!! I am going to focus on merging this today give we are a week out BUT we can always revisit all of this content and update/ enhance / beautify, etc in the future!! 🚀

thank you for this feedback!!

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

clean-modular-code/intro-clean-code.md Outdated Show resolved Hide resolved
clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
Here’s an example following PEP 8 conventions:

```python
import os
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's alphabetize these imports.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isort makes it a little hard to know what the rules are exactly but if we want to add the sorting rules in here, here are the most common rules (in addition to grouping and alphabetization):

  • modules:
    • module-only imports (import x) come before imports with objects (from x import y)
    • within that, import statements sorted alphabetically by module name
  • objects within a module import (the y in from x import y)
    • first sort by whether the first letter is capitalized or not
    • then alphabetically

clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sneakers-the-rat sneakers-the-rat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK Leah, love it, and i hadn't seen the rest of this so i got carried away reading around the rest and i love that too. I think my role in reviewing your writing is always just "take all the really good stuff here and suggest totally switching the order around," which is hopefully clear at this point that you can always take or leave, totally up to you. I have been going through this recently with some new labbies who are just getting booted into collaborative coding for the first time so this is very top of mind for me and wished i had a resource like this!

One thing i'm missing here is an overview of "what are the kinds of things that might be subject to style rules." The examples are good, but some bigger picture categories that i could group things into: naming, comments, whitespace, character use (tabs/spaces, '/"), syntax (eg. using a ternary rather than if/else, etc.). I think reordering that section, adding subheadings between each of them, would make it easier to digest :)

Current structure:

- Why code style and Python PEP 8 matters
- How to Apply PEP 8 Code Style Standards
  - For .py Files
  - For Jupyter Notebooks
  - Running These Tools
- Why use code standards when writing Python code
  - Some PEP 8 rules to remember
- PEP 8 naming conventions
- Best practices for importing libraries
  - Import Python libraries at the top of your code
  - Organize your imports into groups
  - Why organize imports?  

So this is how i would do it (but as always not saying you have to):

- What is code style
  - introductory example (the formatted/unformatted sentence & code examples) 
  - why does it matter for code (code is read more than it's written, merges, etc. all the reasons)
- Common python conventions
  - Formatting (big, top-level, "how is the document structured")
    - Whitespace
    - Imports & Order  
  - Naming (medium, single objects)
    - Capitalization
    - Special characters like l/l
  - Annotations (Notes to self!)
    - Comments
    - Docstrings  
- Tooling
  - what are linters & formatters?
  - examples
    - .py files
    - notebooks
  - pre-commit (why you would do it and links to other guides on how to do it)
  - IDE integration (links to examples)

One other sorta tangential thing- think it's important to give the other side so this too, that they shouldn't feel restricted by linters/formatters! they should use them if they support their work and help them think clearly. If they find a rule annoying? they should turn it off. it's their call. i don't mean to be annoying or overly ideological about this since i am known to make a big deal about standardization in devops but figured i'd bring it up.

@@ -68,7 +65,7 @@ After completing this lesson, you will be able to:
* Apply the PEP 8 Style Guide standards to your **Python** code.
:::

"Pythonic" code is code that follows the conventions and best practices of the Python programming language. It emphasizes code that is clear, concise, and readable--principles that adhere to Python's design philosophy. <link to zen of python>
"Pythonic" code is code that follows the conventions and best practices of the Python programming language. It emphasizes code that is clear, concise, and readable--principles that adhere to Python's design philosophy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Pythonic" code is code that follows the conventions and best practices of the Python programming language. It emphasizes code that is clear, concise, and readable--principles that adhere to Python's design philosophy.
"Pythonic" code is code that follows the conventions and best practices of the Python programming language. It emphasizes code that is clear, concise, and readable--principles that adhere to Python's design philosophy.[^zenofpython]

and at the bottom

[^zenofpython]: Paste this into your shell: `python -c 'import this'`

@@ -14,237 +14,279 @@ kernelspec:

+++ {"editable": true, "slideshow": {"slide_type": ""}}

# Use PEP 8 To Write Easier-to-Read Code
# Python code style for readability and usability
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should have pep-8 in the title if it's going to be in the url.
"Python style conventions, PEP-8 and friends" something like that. or else change the name of the file to be code-style


Code syntax standards refer to rules associated with how code is formatted. These rules can including things like:
Just like good grammar makes text easier to read, PEP 8 helps make your code easier to understand. It enforces rules on naming variables, capitalization, formatting code, and structuring your script. Well-formatted code also makes it easier for you to share code, given it will be easier for others to understand.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be easier for newbies to keep some of the old way of introducing this, "what is code style" and then "what is PEP 8" in sequence. So "what is code style and why should I care," and then "PEP-8 is one/the main/the most common set of python code style guidelines." We're dropping in a little hard here where I am not sure what PEP-8 is at the same time i'm trying to hear what you're saying about why style is important

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think it's worth expanding this a bit before getting into the examples - some other things i think are worth covering from the top:

  • that pep8 is just one example, that there are a ton of style guides and sub-rules and preferences. There isn't "one right python," but there are some generally agreed upon styles that one would consider idiomatic (or pythonic) python as well as a lot of gray area around the edges. we're going to talk about pep-8 in this lesson because it's the most common. hint at this at the top and then we can give some more examples at the bottom for further reading
  • the other major reason to use (and enforce) a code style is that is makes collaboration on large projects a lot less of a pain in the ass. rather than having to wade through 500 lines where someone nudged some whitespace, or getting merge conflicts from the absence of a trailing comma, there is one unambiguous way that the code should be (modulo all the stylistic space that the style guide doesn't regulate)
  • Linters also make code easier to write - i often find myself just writing sludgy garbage and then saving and letting the linter figure it out for me. Code style makes it so you don't have to think about the formatting as you write.
  • Another is that consistent style makes bugs in code easier to recognize. If you're not bogged down by having to constantly adjust to the 50 different ways that the same pattern is written, you can see what the code is doing more clearly. bugs can "look wrong" when you have a clear expectation of what the code "should" look like.
  • It also gives external contributors a clear target to aim for. Rather than someone having to manually review and say 'in this project we do this not that,' or outright rejecting a PR because it's ugly (to the maintainer), clear code style guidelines give an unambiguous thumbs up/thumbs down on whether a contribution is adherent to the style.
  • I think it would also be good to add a footnote or a breakout box or something like "don't worry about your code being perfect. the goal of linting tools is to reduce stress not add to it - there are still many "correct" ways to write things, and your code is not bad if it is not adherent to the styles"

clean-modular-code/python-pep-8.md Outdated Show resolved Hide resolved
import pandas as pd
from datetime import datetime
def classify_precipitation(precip_list):
avg_precip=pd.Series(precip_list).mean()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the single space is killing me

## Summary -- PEP 8 and Python

The text above provides a broad overview of some of the PEP 8 guidelines and conventions for writing **Python** code. It is not fully inclusive all of all the standards which are included in the full, online PEP 8 documentation.
## PEP 8 naming conventions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is odd to me to have as its own top-level section after listing a bunch of pep 8 rules in the previous subsection, and it seems to be the same information as the example starting on line 179? i would combine those things and put them under a section on various types of conventions. i'll give an example ToC in the main comment


Other linters will simply check your code and tell you if things need to be fixed. A few **Python** packages that perform linting are listed below.
First, let's review some terminology associated with naming conventions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

section starts "first" but there is no next!


import matplotlib.pyplot as plt
import numpy as np
import pandas as pd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hear you on that but i think that currently it's a little bewildering the other way where we're missing some introductory information on what we're talking about and why first, about to post the review

edit: oh wait it makes this comment part of the review even when we started it before that lol my b

Here’s an example following PEP 8 conventions:

```python
import os
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isort makes it a little hard to know what the rules are exactly but if we want to add the sorting rules in here, here are the most common rules (in addition to grouping and alphabetization):

  • modules:
    • module-only imports (import x) come before imports with objects (from x import y)
    • within that, import statements sorted alphabetically by module name
  • objects within a module import (the y in from x import y)
    • first sort by whether the first letter is capitalized or not
    • then alphabetically


* <a href="https://www.safaribooksonline.com/library/view/the-hitchhikers-guide/9781491933213/ch04.html" target="_blank">The Hitchhiker's Guide to Python by Tanya Schlusser and Kenneth Reitz</a>
Organizing your imports this way ensures your code is readable and follows widely accepted practices. Importing libraries at the top also makes it easier to debug and see which dependencies are required to run the code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and merge conflicts, but i think basically we should consolidate "why code style" at the beginning because it's mostly the same reasons

@willingc
Copy link
Contributor

@lwasser @sneakers-the-rat Let's split this PR up or merge as is and then do another iteration. I'm finding it difficult to work with all three files in the same PR and follow proposed edits.

I would love to see something more along the lines:

  • Create maintainable code
    • Why it's helpful
    • What are elements of maintainable code
      • Clean
      • Modular
      • Readable
      • Consistent code style
      • DRY: minimize code repetition
  • Code style
    • What is code style
    • Why it's important
      • Easier to maintain
      • Easier to change and review
    • Types of tools (generic)
      • Linters
      • Formatters
      • Integrated into editors, IDEs, CI
  • Python code style: PEP8 and beyond
    • PEP8
    • black

Etc.

@willingc
Copy link
Contributor

@lwasser Let's merge this as is and iterate on separate PRs.

@lwasser
Copy link
Member Author

lwasser commented Oct 17, 2024

Ok fantastic yall. Let's do this

  1. When I'm back at.my computer I'll turn the bigger comments into issues!
  2. I already committed the online edits!
  3. I'll merge so we can then iterate on smaller prs. This likely will happen after the fall festival but I'm excited to work on this with everyone!! And I'll keep each pr small from here on in..

I hope that works for you both!! ✨️

@lwasser lwasser merged commit 9459175 into pyOpenSci:main Oct 17, 2024
3 checks passed
@lwasser lwasser deleted the linters branch October 17, 2024 23:18
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.

3 participants