Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
anenadic authored Oct 11, 2024
1 parent db5b7c8 commit c6daf57
Show file tree
Hide file tree
Showing 19 changed files with 41 additions and 41 deletions.
2 changes: 1 addition & 1 deletion _episodes/15-coding-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ Avoid extraneous whitespace in the following situations:
augmented assignment (+=, -= etc.),
comparisons (==, <, >, !=, <>, <=, >=, in, not in, is, is not),
booleans (and, or, not).
- do not use spaces around the = sign
- Do not use spaces around the = sign
when used to indicate a keyword argument assignment
or to indicate a default value for an unannotated function parameter
~~~
Expand Down
2 changes: 1 addition & 1 deletion _episodes/16-verifying-code-style-linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ see the "W0611: Unused numpy imported as np (unused-import)" warning.

It is important to note that while tools such as Pylint are great at giving you
a starting point to consider how to improve your code,
they Will not find everything that may be wrong with it.
they will not find everything that may be wrong with it.

> ## How Does Pylint Calculate the Score?
>
Expand Down
4 changes: 2 additions & 2 deletions _episodes/21-automatically-testing-software.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ to test that our calculated result is the same as our expected result.
This function explicitly checks the array's shape and elements are the same,
and throws an `AssertionError` if they are not.
In particular, note that we cannot just use `==` or other Python equality methods,
since these Will not work properly with NumPy arrays in all cases.
since these will not work properly with NumPy arrays in all cases.

We could then add to this with other tests that use and test against other values,
and end up with something like:
Expand Down Expand Up @@ -416,7 +416,7 @@ but what you learn can scale to more complex functional testing for applications
>
> Other unit testing frameworks exist for Python,
> including Nose2 and Unittest, with Unittest supplied as part of the standard Python library.
> it is also worth noting that Pytest supports tests written for Unittest,
> It is also worth noting that Pytest supports tests written for Unittest,
> a useful feature if you wish to prioritise use of the standard library initially,
> but retain the option to move Pytest in the future.
>
Expand Down
2 changes: 1 addition & 1 deletion _episodes/23-continuous-integration-automated-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ jobs:

***Note**: be sure to create this file as `main.yml`
within the newly created `.github/workflows` directory,
or it Will not work!*
or it will not work!*

So as well as giving our workflow a name - CI -
we indicate with `on` that we want this workflow to run when we `push` commits to our repository.
Expand Down
4 changes: 2 additions & 2 deletions _episodes/24-diagnosing-issues-improving-robustness.md
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ so this will clearly break if we are dividing by zero here,
resulting in `NaN` values in the normalised array.

With all this in mind,
Let us add a few edge cases to our parametrisation of `test_patient_normalise`.
let us add a few edge cases to our parametrisation of `test_patient_normalise`.
We will add two extra tests,
corresponding to an input array of all 0,
and an input array of all 1.
Expand Down Expand Up @@ -821,7 +821,7 @@ do not forget to remove the `--fail-under` argument to Pytest
in our GitHub Actions configuration file too,
since we do not need it anymore.
Now when we run Pylint we Will not be penalised for having a reasonable line length.
Now when we run Pylint we will not be penalised for having a reasonable line length.
For some further hints and tips on how to approach using Pylint for a project,
see [this article](https://pythonspeed.com/articles/pylint/).
Expand Down
2 changes: 1 addition & 1 deletion _episodes/33-code-decoupling-abstractions.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ In the code from our current branch `full-data-analysis`,
you may have noticed that loading data from CSV files from a `data` directory is "hardcoded" into
the `analyse_data()` function.
Data loading is a functionality separate from data analysis, so firstly
Let us decouple the data loading part into a separate component (function).
let us decouple the data loading part into a separate component (function).

> ## Exercise: Decouple Data Loading from Data Analysis
>
Expand Down
2 changes: 1 addition & 1 deletion _episodes/43-software-release.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ make sure you have got the correct virtual environment activated.

Poetry can also handle virtual environments for us,
so in order to behave similarly to how we used them previously,
Let us change the Poetry config to put them in the same directory as our project:
let us change the Poetry config to put them in the same directory as our project:

~~~ bash
$ poetry config virtualenvs.in-project true
Expand Down
4 changes: 2 additions & 2 deletions _episodes/51-managing-software.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ The [default labels available in GitHub](https://docs.github.com/en/issues/using
- `help wanted` - indicates that a maintainer wants help on an issue or pull request
- `invalid` - indicates that an issue, pull request, or discussion is no longer relevant
- `question` - indicates that an issue, pull request, or discussion needs more information
- `wontfix` - indicates that work Will not continue on an issue, pull request, or discussion
- `wontfix` - indicates that work will not continue on an issue, pull request, or discussion

You can also create your own custom labels to help with classifying issues.
There are no rules really about naming the labels -
Expand Down Expand Up @@ -122,7 +122,7 @@ the easier the code is to use, the more widely it will be adopted
and the greater impact it will have.

One interesting label is `wontfix`,
which indicates that an issue simply Will not be worked on for whatever reason.
which indicates that an issue simply will not be worked on for whatever reason.
Maybe the bug it reports is outside of the use case of the software,
or the feature it requests simply is not a priority.
This can make it clear you have thought about an issue and dismissed it.
Expand Down
6 changes: 3 additions & 3 deletions _episodes/52-assessing-software-suitability-improvement.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ objectives:
- "Conduct an assessment of software against suitability criteria"
- "Describe what should be included in software issue reports and register them"
keypoints:
- "it is as important to have a critical attitude to adopting software as we do to developing it."
- "It is as important to have a critical attitude to adopting software as we do to developing it."
- "As a team agree on who and to what extent you will support software you make available to others."
---

Expand Down Expand Up @@ -83,14 +83,14 @@ will help you adopt the same attitude when assessing your own software for futur
> - Manage your support:
> an issue tracker - like the one in GitHub - is essential to track and manage issues
> - Manage expectations:
> Let users know the level of support you offer,
> let users know the level of support you offer,
> in terms of when they can expect responses to queries,
> the scope of support (e.g. which platforms, types of releases, etc.),
> the types of support (e.g. bug resolution, helping develop tailored solutions),
> and expectations for support in the future (e.g. when project funding runs out)
>
> All of this requires effort, and you cannot do everything.
> it is therefore important to agree and be clear on
> It is therefore important to agree and be clear on
> how the software will be supported from the outset,
> whether it is within the context of a single laboratory,
> project,
Expand Down
10 changes: 5 additions & 5 deletions _episodes/53-improvement-through-feedback.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ MoSCoW is an acronym that stands for
**Must have**,
**Should have**,
**Could have**,
and **Will not have**.
and **Won't have**.
Each requirement is discussed by the stakeholder group and falls into one of these categories:

- *Must Have* (MH) -
Expand All @@ -164,7 +164,7 @@ Each requirement is discussed by the stakeholder group and falls into one of the
- *Could Have* (CH) -
these are desirable but not necessary,
and each of these will be included in this timebox if it can be achieved.
- *Will not Have* (WH) -
- *Won't Have* (WH) -
these are agreed to be out of scope for this timebox,
perhaps because they are the least important or not critical for this phase of development.

Expand All @@ -187,7 +187,7 @@ you have still delivered it *successfully*.

### GitHub's Milestones

Once we have decided on those we will work on (i.e. not Will not Haves),
Once we have decided on those we will work on (i.e. not Won't Haves),
we can (optionally) use a GitHub's **Milestone** to organise them for a particular timebox.
Remember, a milestone is a collection of issues to be worked on in a given period (or timebox).
We can create a new one by selecting `Issues` on our repository,
Expand Down Expand Up @@ -216,7 +216,7 @@ Let us now use Milestones to plan and prioritise our team's next sprint.
> Put your stakeholder hats on, and as a team apply MoSCoW to the repository issues
> to determine how you will prioritise effort to resolve them in the allotted time.
> Try to stick to the 60/20/20 rule,
> and assign all issues you'll be working on (i.e. not `Will not Haves`) to a new milestone,
> and assign all issues you will be working on (i.e. not `Won't Haves`) to a new milestone,
> e.g. "Tidy up documentation" or "version 0.1".
>
>
Expand Down Expand Up @@ -257,7 +257,7 @@ and serves to highlight any blockers and challenges to meeting the sprint goal.
{: .challenge}

Depending on how many issues were registered on your repository,
it is likely you Will not have resolved all the issues in this first milestone.
it is likely you will not have resolved all the issues in this first milestone.
Of course, in reality, a sprint would be over a much longer period of time.
In any event, as the development progresses into future sprints
any unresolved issues can be reconsidered and prioritised for another milestone,
Expand Down
2 changes: 1 addition & 1 deletion _extras/common-issues.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ This issue seems to only occur with older versions of PyCharm - recent versions
### Invalid YAML Issue
If YAML is copy+pasted from the course material,
it might not get pasted correctly in PyCharm and some extra indentation may occur.
Annoyingly, PyCharm Will not flag this up as invalid YAML
Annoyingly, PyCharm will not flag this up as invalid YAML
and learners may get all sort of different issues and errors with these files -
e.g. ‘actions must start with run or uses’ with GitHub Actions workflows.
Expand Down
12 changes: 6 additions & 6 deletions _extras/databases.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,13 @@ Some examples of databases which are used like this are PostgreSQL, MySQL and MS

On the other hand, SQLite runs entirely within our software
and uses only a single file to hold its data.
It Will not give us
It will not give us
the extremely high performance or reliability of a properly configured PostgreSQL database,
but it is good enough in many cases and much less work to get running.

Let us write some test code to setup and connect to an SQLite database.
For now we will store the database in memory rather than an actual file -
it Will not actually allow us to store data after the program finishes,
it will not actually allow us to store data after the program finishes,
but it allows us not to worry about **migrations**.

> ## Migrations
Expand All @@ -157,7 +157,7 @@ but it allows us not to worry about **migrations**.
> will compare our mappings to the known state of the database
> and generate a Python file which updates the database to the necessary state.
>
> Migrations can be quite complex, so we Will not be using them here -
> Migrations can be quite complex, so we will not be using them here -
> but you may find it useful to read about them later.
{: .callout}

Expand Down Expand Up @@ -393,8 +393,8 @@ def test_sqlalchemy_observations_to_array():

> ## Refactoring for Reduced Redundancy
>
> you have probably noticed that there is a lot of replicated code in our database tests.
> it is fine if some code is replicated a bit,
> You have probably noticed that there is a lot of replicated code in our database tests.
> It is fine if some code is replicated a bit,
> but if you keep needing to copy the same code,
> that's a sign it should be refactored.
>
Expand Down Expand Up @@ -426,7 +426,7 @@ def test_sqlalchemy_observations_to_array():
> show an existing record,
> update an existing record
> and delete an existing record.
> it is also sometimes useful to provide a view which lists all existing records for each type -
> It is also sometimes useful to provide a view which lists all existing records for each type -
> for example, a list of all patients would probably be useful,
> but a list of all observations might not be.
>
Expand Down
2 changes: 1 addition & 1 deletion _extras/functional-programming.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ print(list(squares))
> but at that point we should be using a ‘normal’ Python function instead.
>
> ~~~
> # do not do this
> # Do not do this
> add_one = lambda x: x + 1
>
> # Do this instead
Expand Down
6 changes: 3 additions & 3 deletions _extras/object-oriented-programming.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ patients = [
> > > A better solution would be to use the `zip` function,
> > > which allows us to iterate over multiple iterables without needing an index variable.
> > > The `zip` function also limits the iteration to whichever of the iterables is smaller,
> > > so we Will not raise an exception here,
> > > so we will not raise an exception here,
> > > but this might not quite be the behaviour we want,
> > > so we will also explicitly `assert` that the inputs should be the same length.
> > > Checking that our inputs are valid in this way is an example of a precondition,
Expand Down Expand Up @@ -515,7 +515,7 @@ parameterising unit tests and functional programming -
In this case the `property` decorator is taking the `last_observation` function
and modifying its behaviour,
so it can be accessed as if it were a normal attribute.
It is also possible to make your own decorators, but we Will not cover it here.
It is also possible to make your own decorators, but we will not cover it here.
## Relationships Between Classes
Expand Down Expand Up @@ -684,7 +684,7 @@ who is a Person but not a Patient.
We see in the example above that to say that a class inherits from another,
we put the **parent class** (or **superclass**) in brackets after the name of the **subclass**.
there is something else we need to add as well -
There is something else we need to add as well -
Python does not automatically call the `__init__` method on the parent class
if we provide a new `__init__` for our subclass,
so we will need to call it ourselves.
Expand Down
2 changes: 1 addition & 1 deletion _extras/persistence.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ Since this new serializer is not a type of `PatientSerializer`,
we need to inherit from a new base class
which holds the design that is shared between `PatientSerializer` and `ObservationSerializer`.
Since we do not actually need to save the observation data to a file independently,
we Will not worry about implementing the `save` and `load` methods for the `Observation` model.
we will not worry about implementing the `save` and `load` methods for the `Observation` model.

~~~
# file: inflammation/serializers.py
Expand Down
2 changes: 1 addition & 1 deletion _extras/programming-paradigms.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ With datasets like this, we cannot move the data around easily,
so we often want to send our code to where the data is instead.
By writing our code in a functional style,
we also gain the ability to run many operations in parallel
as it is guaranteed that each operation Will not interact with any of the others -
as it is guaranteed that each operation will not interact with any of the others -
this is essential if we want to process this much data in a reasonable amount of time.

You can read more in an [extra episode on Functional Programming](/functional-programming/index.html).
Expand Down
4 changes: 2 additions & 2 deletions slides/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ python3 -m venv .venv # it is important to use the dot prefix if you are creati
. .venv/bin/activate
pip install -r slides/requirements.txt
# launch jupyter from the top level of this repo, **not** in the slide
# directory or else the relative figure links Will not work
# directory or else the relative figure links will not work
jupyter-notebook
# navigate to the slide file and edit
```
Expand All @@ -28,7 +28,7 @@ Use spacebar to advance slides. Presenter view with `t`.

Saving the slides from the Jupyter interface should only save to the markdown source file.
If you find you have ended up with some `.ipynb` files in the `slides/` directory,
then you have done something wrong. do not check those `.ipynb` files into version control.
then you have done something wrong. Do not check those `.ipynb` files into version control.

## Slide Export

Expand Down
6 changes: 3 additions & 3 deletions slides/section_3_software_dev_process.md
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ Pure functions are also easier to test

* Easier to write as can create the input as we need it
* Easier to read as do not need to read any external files
* Easier to maintain - tests Will not need to change if the file format changes
* Easier to maintain - tests will not need to change if the file format changes

<!-- #endregion -->

Expand Down Expand Up @@ -690,7 +690,7 @@ Time: 15min

These are techniques from **object oriented programming**.

There is a lot more that we Will not go into:
There is a lot more that we will not go into:

* Inheritance
* Information hiding
Expand Down Expand Up @@ -830,7 +830,7 @@ Practise makes perfect:

* Spot signs things could be improved - like duplication
* Think about why things are working or not working
* do not design for an imagined future
* Do not design for an imagined future
* Keep refactoring as you go

<!-- #endregion -->
Expand Down
8 changes: 4 additions & 4 deletions slides/section_4_collaborative_soft_dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ Go through the steps described under heading. Stop when you reach **Reviewing a

Pair up with someone else in your group and exchange repository links. You will be taking on the role of _Reviewer_ on your partner's repository. Before leaving review comments, read the content under the heading **Reviewing a pull request**. Try to make a comment from each of the main areas identified.

**do not submit your review just yet!!!**
**Do not submit your review just yet!!!**
<!-- #endregion -->

<!-- #region slideshow={"slide_type": "notes"} -->
Expand All @@ -166,7 +166,7 @@ When done, select `Request changes` from the list of toggles, then `Submit revie

Respond to the _Reviewers_ comments on the PR in _your_ repository. Use the information in **Responding to review comments** to guide your responses. And remember that you can talk to your _Reviewer_ for clarification, just make sure you record that in a comment on the PR.

do not implement changes that will take more than 5 minutes. Instead, raise them as an issue on your repo for future work, and link to that issue in a comment on the PR.
Do not implement changes that will take more than 5 minutes. Instead, raise them as an issue on your repo for future work, and link to that issue in a comment on the PR.
<!-- #endregion -->

<!-- #region slideshow={"slide_type": "notes"} -->
Expand Down Expand Up @@ -235,7 +235,7 @@ Start from the top of this episode page and go to the end.
<!-- #region slideshow={"slide_type": "notes"} -->
- Learners can skim the first two sections if you have talked about them in the previous slide
- Split into breakout rooms for about 50 minutes
- A preface note: if you have been using codimd or hackmd for the shared document, then learners will have already been exposed to Markdown, so this section Will not contain much new for them
- A preface note: if you have been using codimd or hackmd for the shared document, then learners will have already been exposed to Markdown, so this section will not contain much new for them
- Post episode comments
- A README is a great place to start your documentation, but at some point it will outgrow that, and you will need a bigger documentation system. The most popular in Python is Sphinx, which can be used with Markdown or another markup language called ReStructuredText (`.rst` files)
- For writing documentation, this is another great link that can be added to the shared document: https://documentation.divio.com/
Expand Down Expand Up @@ -363,7 +363,7 @@ Inspect how `pyproject.toml` has changed. Look at what has gone into `poetry.loc
- Give learners a few minutes to do this
- Then, quickly run through it and look at the changes in your own repo
- Explain when `poetry.lock` is used: if it is present in a repo when `poetry install` is called, the _exact_ versions of dependencies in `poetry.lock` will be used. Again, useful if we are distributing a standalone application.
- do not check in `poetry.lock` into version control if you want your package to be used as a library
- Do not check in `poetry.lock` into version control if you want your package to be used as a library
- Note that the last command is quite important because it puts the current package we are developing into our environment
- This means that some of those annoying `ModuleNotFound` errors will be eliminated
- Generally, we want to install the package we are working on in our environment
Expand Down

0 comments on commit c6daf57

Please sign in to comment.