Conversation
… programming practices
fwimp
requested changes
Oct 27, 2024
Collaborator
fwimp
left a comment
There was a problem hiding this comment.
Introduction
- I'm a little confused here, this intro seems to imply that students will complete both Python weeks, but also the pull request implies that these two have been merged.
Why Python?
- It may be worthwhile pointing out here that in a lot of cases (ESPECIALLY IN THIS JULIA-PRODUCED GRAPH) you can actually make python run a lot faster than it seems here. You just need to code python in the way it should be written, not like you're writing a different language (as they sorta did here).
Python versions
- Is it true that most of the code will work in Python 2.7? I don't think so, can probably remove this.
Determining an object's type
- The
geditcommand here seems like a throwback to the original silbiocomp repo, why not usenano? Or justcode?
Assigning and manipulating variables
- Again here, multi-line commands should be formatted in proper triple-backtick-fenced code blocks rather than individual cells.
Python data structures
- Table is missing headers
- Also weird italicisation in the first column
Lists
- Again with these multi-line commands in separate cells!
Sets
- You can also convert tuples to sets!
Dictionaries
- It may be worthwhile explicitly spelling out here that every dictionary key must be unique.
Copying mutable objects
- Double comma in first sentence.
Testing/Running blocks of code
- Weird star as a hangover from silbiocomp
Functions
- Some output here seems to be missing
Conditionals
- Erm... 0 is most certainly an integer!
- What we should be talking about is that the truthiness of an integer is dependent on its value, where anything other than 0 is cast to
True, with 0 beingFalse - This is actually misleading so definitely needs to be changed.
Variable scope
- Spelling error: "seuccessive simmation" should be "successive summation"
Importance of the return directive
- "returning a None" note box is weirdly formatted.
Handling csv’s
- Weird stars throughout this as a remnant of silbiocomp
Writing Python programs
- Weird stars here too
The Docstring
- It may be worthwhile linking to a standard form for docstrings here, such as the google style.
Internal Variables
- Technically it's fine to define your own dunder variables, in fact it's sometimes a good thing to do (such as when defining versioning or especially when writing your own classes). It may be better to explain a bit more about what they do and why they're there.
- This is especially pertinent as in the "A program-with-control-flows example" section the example uses dunder variables in the first 7 lines...
What is sys.argv?
- It may be worthwhile pointing students towards
argparsefor this, as they definitely should know about it.
Errors that cannot be debugged
- Code in the tip box is not correctly formatted. Should use a triple backtick code fence.
Collaborator
Author
|
Thanks for the very detailed review, @fwimp ! That's a long list of changes, so am going to deploy the book with some edits and then address these... |
Collaborator
|
Lots of suggestions, but most of them shouldn't require much thought to investigate :) |
… as a separate notebook for future testing and re-integration
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@fwimp , can you please review this chapter, specifically the new section of good programming practices?