Skip to content

FIX: seven source-side defects in python_by_example, functions, numpy and pandas - #604

Merged
mmcky merged 2 commits into
mainfrom
fix-602-source-defects
Aug 5, 2026
Merged

FIX: seven source-side defects in python_by_example, functions, numpy and pandas#604
mmcky merged 2 commits into
mainfrom
fix-602-source-defects

Conversation

@mmcky

@mmcky mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Addresses #602. Each of the eight reports was validated against the source before anything was changed. Seven are actioned here and one is a false positive, left alone with the reasoning below. Two of the seven (items 1 and 2) turned out to be style and readability improvements rather than the defects they were reported as — noted individually below.

What changed

# File Fix Verdict
1 python_by_example.md Four inline spans written with triple backticks now use single backticks Cosmetic — see note
2 functions.md "a First In Last Out (FILO) queue" → "a last-in, first-out (LIFO) data structure" Wording — see note
3 numpy.md Space added after the --- marker at line 218 Confirmed, wording of the report inverted
4 numpy.md NumpyNumPy; **Part1****Part 1**; and **Part2****Part 2** Confirmed, plus one the report missed
5 numpy.md Prose discreteRVDiscreteRV Confirmed
6 pandas.md No change — the query is correct as written False positive
7 pandas.md Dead research.stlouisfed.org URL removed from the prose Confirmed, 301 → 404
8 pandas.md type(x) != strnot isinstance(x, str); np.isnanpd.isna Confirmed

Item 6 is a false positive — the query is correct

The report claims that df.query("cc + cg >= 80 & POP <= 20000") is not equivalent to the parenthesised boolean-indexing example above it, because & binds tighter than the comparisons. That is true of plain Python, but it is not true inside DataFrame.query.

With the default parser='pandas', pandas runs a token-level preparse (_replace_booleans in pandas/core/computation/expr.py) that rewrites the & token to and before the string reaches ast.parse. Its docstring says so explicitly: "Replace & with and and | with or so that bitwise precedence is changed to boolean precedence." The expression therefore parses with boolean precedence and needs no parentheses.

Verified empirically on the lecture's own dataset — both forms return the same two rows (Malawi, Uruguay) and .equals() is True. Instrumenting the live query() call shows the expression arriving at the parser as cc +cg >=80 and POP <=20000, i.e. already rewritten. The surrounding prose ("the above is equivalent to") is accurate, so nothing here needs changing.

Item 1 is a real inconsistency but not a rendering bug

The report's stated reason — "triple backticks are fence syntax" — does not hold for a span that opens and closes mid-paragraph. Per CommonMark a fence must begin at the start of a line, and a code span may use any number of backticks as long as the runs match, so these render as <code> today under both myst-parser 3.0.1 and mystmd 1.10.1. The rendered HTML before and after this change is byte-identical.

The change is still worth making: the file has 60 single-backtick spans against these 4, and translated editions copy the source verbatim. Treat it as source hygiene, not a bug fix.

Item 2 is a readability change, not an error

The report calls "a First In Last Out (FILO) queue" wrong on the grounds that "the standard term is Last In, First Out (LIFO)". The two are equivalent: first-in-last-out and last-in-first-out describe the same access discipline from opposite ends, so the original sentence was not incorrect. Nor is "queue" a category error — Python's stdlib ships queue.LifoQueue, documented as "retrieves most recently added entries first", and asyncio.LifoQueue alongside it.

The change is still worth making, but on readability grounds rather than correctness: LIFO is the term students meet in CS curricula and in Python's own docs, and FILO is a real but uncommon synonym. "data structure" over "queue" avoids a beginner reading "queue" in its ordinary FIFO sense. Happy to drop this one if you'd rather leave the original wording alone.

Item 3's wording is inverted, but there is a real defect at that line

The report says "no space before the em-dash marker". There is a space before it; the missing one is after. A survey of all 86 mid-sentence --- uses across lectures/ gives 79% space-on-both-sides, 16% closed up on both sides, and exactly one hybrid — this line. It is also the only asymmetric case in numpy.md, whose other two instances (lines 133 and 448) use the spaced form. Fixed by adding the missing space.

Item 4 was incomplete as reported

The report lists **Part1** at line 1413. The same exercise has **Part2** at line 1432, whose matching solution heading at line 1502 already reads **Part 2 Solution**. Fixing only the reported line would have left the pair inconsistent, so both are fixed.

Verification

The two edited code cells were replayed against test_pwt.csv in the lecture's exact sequence, comparing the current and proposed versions cell by cell:

Cell repr identical .equals() dtypes identical
df.map(round…) yes yes yes
df.map(replace_nan) yes yes yes
downstream fillna yes yes yes

So no rendered notebook output moves and no cached execution is invalidated by the behaviour change. The breakage the report describes does reproduce: type(x) != str sends str subclasses and np.str_ into round(), and np.isnan raises TypeError on None, pd.NA, pd.NaT and arbitrary objects where pd.isna returns cleanly.

All four files were also parsed before and after with the project's myst-parser 3.0.1 and the repo's extension set. Token counts and structure are unchanged in every file; only the intended inline/fence tokens differ.

The dead FRED URL was re-checked live: research.stlouisfed.org/fred2/series/UNRATE/downloaddata/UNRATE.csv returns 301 to fred.stlouisfed.org/data/UNRATE.csv, which returns 404. The fredgraph.csv URL used by the code cells returns 200. Rather than inline that ~700-character query string into prose as a third copy, the sentence now names requests.get(url), matching the cell immediately below it — so prose and code stay in sync whenever the URL is refreshed.

Folded in after review (bed9f09)

Two pre-existing prose defects on lines this PR already touches. Both were originally listed below as out-of-scope candidates, and both were independently raised in Copilot's review:

  • python_by_example.md:383 — the sentence said a while loop runs "until the condition is satisfied", which is backwards; it runs while the condition holds. It now reads "as long as the condition (i < ts_length) is satisfied". This was contradicting the very next sentence at line 385, which already described the behaviour correctly.
  • functions.md:450 — "each successive call uses it's own frame" → "its". The other four it's in that file (lines 96, 98, 190, 415) are correct contractions of "it is" and were left alone.

Not included — flagged for a separate decision

These came out of a repo-wide sweep for the same defect classes. They are outside the scope of #602, so I have left them alone:

  • workspace.md:273 and :277 — the same triple-backtick inline spans (```Python: Select Interpreter```, ```Python: Create Environment```). These are the only other instances repo-wide.
  • sympy.md:643 — "Sympy" against 21 correct "SymPy" spellings in the same file.
  • sympy.md 357, 359, 361, 428, 453 — prose says Stats module; the import is lowercase sympy.stats. Lower confidence, since the linked SymPy docs page is itself titled "Stats".

Happy to fold any of these in too if you'd rather not open a second PR.

🤖 Generated with Claude Code

Addresses the defects reported in #602, which were surfaced by Copilot's
review of the Malayalam translation PRs and traced back to the English
source.

Code correctness (pandas.md):

* replace `type(x) != str` with `not isinstance(x, str)` in both `.map()`
  examples, so `str` subclasses are treated as strings
* replace `np.isnan(x)` with `pd.isna(x)` in `replace_nan`, which tolerates
  `None`, `pd.NA` and `pd.NaT` where `np.isnan` raises `TypeError`
* drop the dead `research.stlouisfed.org` URL from the prose introducing the
  FRED example; it now names `requests.get(url)`, matching the code cell
  directly below it

Both edited cells were replayed against the lecture's own data: the rendered
output and dtypes are byte-identical, so no cached notebook output moves.

Typography and naming:

* python_by_example.md: use single backticks for the four inline code spans
  written with triple backticks
* functions.md: describe the call stack as a last-in, first-out (LIFO) data
  structure rather than a "First In Last Out (FILO) queue"
* numpy.md: space after the `---` marker at line 218, matching the repo's
  dominant convention; `Numpy` -> `NumPy`; `discreteRV` -> `DiscreteRV` to
  match the class actually defined in the cell above

Item 6 of #602 is not actioned: `df.query("cc + cg >= 80 & POP <= 20000")` is
correct as written. `DataFrame.query` rewrites the `&` token to `and` before
parsing, so the expression has boolean precedence and is equivalent to the
parenthesised boolean-indexing example above it, as the prose claims.

`**Part2**` at numpy.md:1432 is also fixed. It is the sibling of the reported
`**Part1**` in the same exercise, and fixing only the reported line would have
left the pair inconsistent.

Closes #602

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 5, 2026 00:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates several lecture sources to correct typos/terminology, improve consistency in inline formatting and naming (e.g., NumPy), and harden pandas examples against common type/missing-value edge cases identified via translation review (#602).

Changes:

  • Normalize inline code spans and fix terminology/wording issues in python_by_example.md and functions.md.
  • Improve consistency in numpy.md (spacing, naming, exercise headings, and a class name reference).
  • Fix robustness and prose accuracy in pandas.md (string detection, missing-value handling, and removal of a dead FRED URL reference).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
lectures/python_by_example.md Converts triple-backtick inline spans to single-backtick inline code in the while-loop explanation.
lectures/functions.md Corrects stack terminology to LIFO phrasing in the recursion/stack explanation section.
lectures/numpy.md Fixes minor spacing and naming consistency issues (NumPy capitalization, Part 1/2 headings, class name reference).
lectures/pandas.md Makes example code more robust (isinstance, pd.isna) and updates prose to avoid referencing a dead URL.

Comment thread lectures/python_by_example.md Outdated
Comment thread lectures/functions.md Outdated
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

@github-actions
github-actions Bot temporarily deployed to pull request August 5, 2026 00:17 Inactive
Two pre-existing prose defects on lines this PR already touches, both raised
in Copilot's review of #604 and listed as out-of-scope candidates in the PR
description.

* python_by_example.md: a while loop runs *as long as* its condition holds,
  not "until the condition is satisfied", which said the loop starts when the
  condition is false and stops when it becomes true. The next sentence already
  described the behaviour correctly, so the two contradicted each other.
* functions.md: "each successive call uses it's own frame" -> "its".

Prose only; no code cells change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions
github-actions Bot temporarily deployed to pull request August 5, 2026 01:15 Inactive
@mmcky

mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Small editorial note on the FILO → LIFO change in functions.md: this one is a suggestion rather than a fix, so it's easy to drop if you'd rather keep the original wording.

FILO and LIFO describe the same discipline, so nothing was wrong as written — the case for switching is just familiarity. Across the Python 3.13 stdlib and every package installed in the quantecon environment, FIFO appears 111 times and LIFO 37; FILO appears zero times. LIFO is the name in queue.LifoQueue and asyncio.LifoQueue, and it turns up in numba, pip, sqlalchemy and twisted as well.

The reason it won is rather neat, and might even earn a parenthetical in the lecture one day: FIFO and LIFO each describe a single element's journey (first in, first out; last in, first out), whereas FILO describes the relationship between two different elements. Same meaning, but it doesn't fit the pattern FIFO sets, so it never caught on.

@mmcky

mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@jstac any preference on the great FILO vs LIFO debate? 😆

@jstac

jstac commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Many thanks @mmcky ! LIFO is fine by me :-)

Please merge when ready.

@mmcky

mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

thanks @jstac

@mmcky
mmcky merged commit aacff38 into main Aug 5, 2026
5 checks passed
@mmcky
mmcky deleted the fix-602-source-defects branch August 5, 2026 02:40
@mmcky

mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

✅ Translation sync completed (zh-cn)

Target repo: QuantEcon/lecture-python-programming.zh-cn
Translation PR: QuantEcon/lecture-python-programming.zh-cn#86
Files synced (4):

  • lectures/functions.md
  • lectures/numpy.md
  • lectures/pandas.md
  • lectures/python_by_example.md

@mmcky

mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

✅ Translation sync completed (fa)

Target repo: QuantEcon/lecture-python-programming.fa
Translation PR: QuantEcon/lecture-python-programming.fa#149
Files synced (4):

  • lectures/functions.md
  • lectures/numpy.md
  • lectures/pandas.md
  • lectures/python_by_example.md

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