Merge pull request 'MIL-004: Create and export a DataFrame' (#29) from mil-004-create-and-export into main
CI / checks (push) Successful in 1m19s
TirSystem/github-action: Sync GitHub mirror metadata / sync-metadata (push) Successful in 8s

This commit was merged in pull request #29.
This commit is contained in:
2026-10-13 09:32:33 +02:00
8 changed files with 368 additions and 3 deletions
+1 -1
View File
@@ -16,7 +16,7 @@ document of a type. `Primary File` may contain a glob (e.g.
| PP | Project Plan | docs/project-plan.md | 002 | | PP | Project Plan | docs/project-plan.md | 002 |
| MIL | Milestone / Gateway | docs/milestones/*.md | 006 | | MIL | Milestone / Gateway | docs/milestones/*.md | 006 |
| DICT | Domain Dictionary | docs/dictionary.md | 002 | | DICT | Domain Dictionary | docs/dictionary.md | 002 |
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 012 | | RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 013 |
## Languages ## Languages
+61
View File
@@ -0,0 +1,61 @@
# RC-012: Code review of MIL-004 (create and export a DataFrame, and the main flow)
## Metadata
| Key | Value |
| --- | --- |
| ID | RC-012 |
| CrossReference | [MIL-004], [QC-PY-001] |
## Version History
| Date | Status | Author | Reviewer | Change | Commit |
| --- | --- | --- | --- | --- | --- |
| 2026-10-13 | Proposed | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [ace3e66] |
---
## Artifact Under Review
- Instance reviewed: the source and tests that [MIL-004] delivers (create and export a DataFrame, and the main flow)
- Checklist used: [QC-PY-001]
- Scope: full review of the files changed on the branch of the milestone
- Language and domain: n/a
- Language reviewer: none
- Mode: autonomous, not independent (the reviewer is `AI as stand-in for S01`; the assistant also wrote the code; the review of the files was done by the `python-developer` agent of the session)
## Checklist Results
| # | Criterion | Status | Evidence/Notes |
| --- | --- | --- | --- |
| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) | Pass | `export`, `create_scores_frame`, `export_frame`, `main`, `EXIT_OK`, `COLUMN_STUDENTS` follow PEP 8 casing; the private helpers start with `_`. |
| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose (`_print_csv_part`, `_print_pandas_part`, `PANDAS_MISSING_MESSAGE`); `data_dict` and `scores` are the lecture's names. |
| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions | Pass | `ruff check` and `ruff format --check` exit 0. The only suppressions are the two `# noqa: E402` after `pytest.importorskip("pandas")` in the test modules, explained by the guard. |
| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | Every function is annotated: `create_scores_frame() -> DataFrame`, `export_frame(...) -> None`, `main(argv: Sequence[str] | None = None) -> int`, the helpers and the tests. |
| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | `main` catches `ImportError` only when `error.name == "pandas"` and re-raises every other one; nothing is swallowed. |
| 6 | No mutable default arguments and no shadowed builtins | Pass | The defaults are immutable (`EXPORT_FILE_NAME`, `None`); no builtin is shadowed. |
| 7 | Files, locks and connections are managed with context managers | N-A | The modules open no file themselves; `DataFrame.to_csv` opens and closes the file. |
| 8 | Public modules, classes and functions have docstrings that say what, not how | Pass | Module docstrings and Doxygen `@brief`, `@param`, `@return`, `@throws` on every function, private helpers included; `doxygen Doxyfile` exits 0 with no warnings. |
| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | Pass | The `print` calls of `__main__` are the program's output, the purpose of the command (as in the lectures); the missing-extra diagnostic goes to stderr; no secret or personal data is printed. There is no logging to replace. |
| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded | Pass | No Design Class Diagram exists (a learning project of this size, recorded in the open issues of [PP-001]); the functions trace to the second lecture and to the tasks of [MIL-004]. |
| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | `tests/test_export.py` and `tests/test_main.py` cover the Go/No-Go criteria of [MIL-004]: the frame's columns and values, the round trip through `tmp_path`, the default name only in the temporary working folder, the printed results, no file without `--export`, the missing-pandas path; `pytest` reports 39 passed; none depends on order or the network. |
| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment | Pass | `python -m mypy` runs with `strict = true`: no issues in 12 source files; no new `Any`. |
| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | No dependency added; `pandas` stays in the extras and is imported only in `analysis` and `export`; `__main__` imports them inside a function so the csv part runs without it. |
| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike | Pass | The lecture's dictionary values, column names, the day and the message are constants in `constants.py`; the export name is defined once. |
| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | `__main__` (delivery) imports `csv_reader`, `analysis`, `export` and `constants`; none of them imports `__main__`; no cycle. |
| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations | Pass | `create_scores_frame`, `export_frame` and `main` each have one reason to change; the printing is split into small helpers; no class hierarchy. |
## Overall Verdict
Go — all Mandatory criteria pass or are not applicable. Assessed on 2026-10-13 by the assistant that wrote the code, as stand-in for S01 in autonomous mode. Mode: autonomous, not independent. The pull request is the second look.
## Action Items
| Action | Owner | Due |
| --- | --- | --- |
| Challenge this `Go` in the run summary or on the pull request if the stand-in decision is not wanted | S01 | 2026-10-23 |
---
[MIL-004]: ../../milestones/mil-004-create-and-export.md
[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
[PP-001]: ../../project-plan.md
[ace3e66]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/025-weather/commit/ace3e66841729c62fd9773f1a2975b9237261f09
+108
View File
@@ -0,0 +1,108 @@
## @file __main__.py
# @brief Command line of the weather data project: `python -m weather`.
#
# Prints the results of both lectures in the order of the lectures. The csv
# part needs no third-party package; the pandas part needs the `pandas` extra.
# A file is written only when `--export PATH` is given.
"""Prints the results of the weather lectures; run with `python -m weather`."""
import argparse
import sys
from collections.abc import Sequence
from pathlib import Path
from weather.constants import MONDAY, PANDAS_MISSING_MESSAGE
from weather.csv_reader import read_rows, read_temperatures
## @brief Value that `main` returns when the run succeeded.
EXIT_OK = 0
## @brief Value that `main` returns when the optional pandas extra is missing.
EXIT_PANDAS_MISSING = 1
def _build_parser() -> argparse.ArgumentParser:
"""! @brief Build the command line parser with its one optional flag.
@return The parser of `python -m weather`.
"""
parser = argparse.ArgumentParser(
prog="weather",
description="Print the results of the weather data lectures.",
)
parser.add_argument(
"--export",
metavar="PATH",
type=Path,
default=None,
help="also write the scores table to this CSV file",
)
return parser
def _print_csv_part() -> None:
"""! @brief Print the rows and temperatures read with the csv library."""
print("Rows (csv library):")
for row in read_rows():
print(row)
print(f"Temperatures (csv library): {read_temperatures()}")
def _print_pandas_part(export_path: Path | None) -> None:
"""! @brief Print the pandas results and export the scores when asked.
@param export_path File to write the scores table to; `None` writes nothing.
@throws ImportError When `pandas` is not installed.
"""
# Imported here so that the csv part runs without the optional extra.
from weather.analysis import (
average_temp,
average_temp_mean,
day_in_fahrenheit,
hottest_row,
max_temp,
read_data,
row_for_day,
to_data_dict,
to_temp_list,
)
from weather.export import create_scores_frame, export_frame
data = read_data()
print(f"Data frame (pandas):\n{data}")
print(f"Data dict: {to_data_dict(data)}")
print(f"Temp list: {to_temp_list(data)}")
print(f"Average temperature (sum / len): {average_temp(data)}")
print(f"Average temperature (mean): {average_temp_mean(data)}")
print(f"Maximum temperature: {max_temp(data)}")
print(f"Row of {MONDAY}:\n{row_for_day(data, MONDAY)}")
print(f"Hottest row:\n{hottest_row(data)}")
print(f"{MONDAY} in Fahrenheit: {day_in_fahrenheit(data, MONDAY)}")
scores = create_scores_frame()
print(f"Scores frame (from a dictionary):\n{scores}")
if export_path is not None:
export_frame(scores, export_path)
print(f"Scores frame exported to: {export_path}")
def main(argv: Sequence[str] | None = None) -> int:
"""! @brief Print the results of the lectures and optionally export a table.
@param argv The command line arguments; `sys.argv[1:]` when `None`.
@return `0` on success, `1` when the pandas extra is not installed.
"""
args = _build_parser().parse_args(argv)
_print_csv_part()
try:
_print_pandas_part(args.export)
except ImportError as error:
if error.name != "pandas":
raise
print(PANDAS_MISSING_MESSAGE, file=sys.stderr)
return EXIT_PANDAS_MISSING
return EXIT_OK
if __name__ == "__main__":
raise SystemExit(main())
+24 -2
View File
@@ -3,8 +3,9 @@
# #
# Every value that the day-25 lectures fix lives here, so that no other module # Every value that the day-25 lectures fix lives here, so that no other module
# contains a magic string or number: the name of the data file, the names of # contains a magic string or number: the name of the data file, the names of
# its columns, the factors of the Fahrenheit conversion and the name of the # its columns, the factors of the Fahrenheit conversion, the name of the
# exported file. # exported file, the table that is created from a dictionary and the message
# for a missing optional dependency.
"""Constants of the weather data project, as fixed by the day-25 lectures.""" """Constants of the weather data project, as fixed by the day-25 lectures."""
@@ -46,3 +47,24 @@ FAHRENHEIT_DENOMINATOR: Final[int] = 5
## @brief Offset of the Celsius to Fahrenheit conversion: `temp * 9 / 5 + 32`. ## @brief Offset of the Celsius to Fahrenheit conversion: `temp * 9 / 5 + 32`.
FAHRENHEIT_OFFSET: Final[int] = 32 FAHRENHEIT_OFFSET: Final[int] = 32
## @brief Name of the column that holds the student names of the lecture's table.
COLUMN_STUDENTS: Final[str] = "students"
## @brief Name of the column that holds the scores of the lecture's table.
COLUMN_SCORES: Final[str] = "scores"
## @brief Student names of the table that the lecture creates from a dictionary.
STUDENTS: Final[tuple[str, ...]] = ("Amy", "James", "Angela")
## @brief Scores of the students, in the order of `STUDENTS`.
SCORES: Final[tuple[int, ...]] = (76, 56, 65)
## @brief Name of the day that the main flow selects and converts.
MONDAY: Final[str] = "Monday"
## @brief Message printed when `pandas`, an optional extra, is not installed.
PANDAS_MISSING_MESSAGE: Final[str] = (
"pandas is not installed; the pandas part needs the extra: "
'pip install "weather[pandas]"'
)
+39
View File
@@ -0,0 +1,39 @@
## @file export.py
# @brief Creates a `DataFrame` from a dictionary and exports it to a CSV file.
#
# This is the end of the second lecture: the table of students and scores is
# built from a dictionary and written with `DataFrame.to_csv`. Nothing is
# written unless `export_frame` is called with a path or its default name.
"""Creates a DataFrame from a dictionary and exports it to a CSV file."""
from pathlib import Path
import pandas
from weather.constants import (
COLUMN_SCORES,
COLUMN_STUDENTS,
EXPORT_FILE_NAME,
SCORES,
STUDENTS,
)
def create_scores_frame() -> pandas.DataFrame:
"""! @brief Create the table of students and scores from a dictionary.
@return A table with the columns `students` and `scores`, one row per student.
"""
data_dict = {COLUMN_STUDENTS: list(STUDENTS), COLUMN_SCORES: list(SCORES)}
return pandas.DataFrame(data_dict)
def export_frame(frame: pandas.DataFrame, path: Path | str = EXPORT_FILE_NAME) -> None:
"""! @brief Write a table to a CSV file, the row index included.
@param frame The table to write.
@param path Path of the file to write; `new_data.csv` in the working folder
by default.
"""
frame.to_csv(path)
+15
View File
@@ -34,3 +34,18 @@ def test_fahrenheit_factors_give_the_lecture_formula() -> None:
def test_exported_file_has_the_name_of_the_lecture() -> None: def test_exported_file_has_the_name_of_the_lecture() -> None:
assert constants.EXPORT_FILE_NAME == "new_data.csv" assert constants.EXPORT_FILE_NAME == "new_data.csv"
def test_scores_table_has_the_columns_and_values_of_the_lecture() -> None:
assert constants.COLUMN_STUDENTS == "students"
assert constants.COLUMN_SCORES == "scores"
assert constants.STUDENTS == ("Amy", "James", "Angela")
assert constants.SCORES == (76, 56, 65)
def test_main_flow_uses_monday() -> None:
assert constants.MONDAY == "Monday"
def test_missing_pandas_message_names_the_extra() -> None:
assert 'pip install "weather[pandas]"' in constants.PANDAS_MISSING_MESSAGE
+52
View File
@@ -0,0 +1,52 @@
"""Tests of the creation and the export of the scores table."""
from pathlib import Path
import pytest
pandas = pytest.importorskip("pandas")
from weather import export # noqa: E402
def test_create_scores_frame_has_the_columns_and_values_of_the_lecture() -> None:
frame = export.create_scores_frame()
assert isinstance(frame, pandas.DataFrame)
assert list(frame.columns) == ["students", "scores"]
assert frame["students"].to_list() == ["Amy", "James", "Angela"]
assert frame["scores"].to_list() == [76, 56, 65]
def test_export_frame_round_trips_through_a_file(tmp_path: Path) -> None:
frame = export.create_scores_frame()
target = tmp_path / "scores.csv"
export.export_frame(frame, target)
assert target.is_file()
read_back = pandas.read_csv(target, index_col=0)
pandas.testing.assert_frame_equal(read_back, frame)
def test_export_frame_accepts_a_string_path(tmp_path: Path) -> None:
target = tmp_path / "as_text.csv"
export.export_frame(export.create_scores_frame(), str(target))
assert target.is_file()
def test_export_frame_default_name_writes_only_new_data_csv_in_the_working_dir(
tmp_path: Path,
) -> None:
# The autouse fixture in conftest.py made tmp_path the working directory.
export.export_frame(export.create_scores_frame())
assert [item.name for item in tmp_path.iterdir()] == ["new_data.csv"]
def test_create_scores_frame_writes_nothing(tmp_path: Path) -> None:
export.create_scores_frame()
assert list(tmp_path.iterdir()) == []
+68
View File
@@ -0,0 +1,68 @@
"""Tests of the command line: the printed results and what is written."""
import sys
from pathlib import Path
import pytest
from weather.__main__ import main
def test_main_without_arguments_prints_the_results_of_the_lectures(
capsys: pytest.CaptureFixture[str],
) -> None:
pytest.importorskip("pandas")
assert main([]) == 0
output = capsys.readouterr().out
assert "17.428" in output
assert "24" in output
assert "Sunny" in output
assert "53.6" in output
assert "Sunday" in output
assert "Angela" in output
def test_main_without_the_flag_writes_no_file(tmp_path: Path) -> None:
pytest.importorskip("pandas")
assert main([]) == 0
assert list(tmp_path.iterdir()) == []
def test_main_with_the_export_flag_writes_the_scores_frame(
tmp_path: Path, capsys: pytest.CaptureFixture[str]
) -> None:
pandas = pytest.importorskip("pandas")
from weather.export import create_scores_frame
target = tmp_path / "out.csv"
assert main(["--export", str(target)]) == 0
assert [item.name for item in tmp_path.iterdir()] == ["out.csv"]
read_back = pandas.read_csv(target, index_col=0)
pandas.testing.assert_frame_equal(read_back, create_scores_frame())
assert str(target) in capsys.readouterr().out
def test_main_reports_the_missing_pandas_extra_and_writes_nothing(
tmp_path: Path,
capsys: pytest.CaptureFixture[str],
monkeypatch: pytest.MonkeyPatch,
) -> None:
# None in sys.modules makes "import pandas" raise ImportError; the modules
# that import it are removed so that they are imported again.
monkeypatch.setitem(sys.modules, "pandas", None)
monkeypatch.delitem(sys.modules, "weather.analysis", raising=False)
monkeypatch.delitem(sys.modules, "weather.export", raising=False)
assert main(["--export", str(tmp_path / "out.csv")]) == 1
captured = capsys.readouterr()
assert 'pip install "weather[pandas]"' in captured.err
assert "Sunny" in captured.out
assert "17.428" not in captured.out
assert list(tmp_path.iterdir()) == []