Merge pull request 'MIL-003: Read and analyse with pandas' (#28) from mil-003-pandas-analysis into main
This commit was merged in pull request #28.
This commit is contained in:
@@ -16,7 +16,7 @@ document of a type. `Primary File` may contain a glob (e.g.
|
||||
| PP | Project Plan | docs/project-plan.md | 002 |
|
||||
| MIL | Milestone / Gateway | docs/milestones/*.md | 006 |
|
||||
| DICT | Domain Dictionary | docs/dictionary.md | 002 |
|
||||
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 011 |
|
||||
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 012 |
|
||||
|
||||
## Languages
|
||||
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
# RC-011: Code review of MIL-003 (the pandas analysis: read_data to day_in_fahrenheit)
|
||||
|
||||
## Metadata
|
||||
| Key | Value |
|
||||
| --- | --- |
|
||||
| ID | RC-011 |
|
||||
| CrossReference | [MIL-003], [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 | [75d7e85] |
|
||||
|
||||
---
|
||||
|
||||
## Artifact Under Review
|
||||
|
||||
- Instance reviewed: the source and tests that [MIL-003] delivers (the pandas analysis: read_data to day_in_fahrenheit)
|
||||
- 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 | `analysis`, `read_data`, `to_data_dict`, `to_temp_list`, `average_temp`, `max_temp`, `row_for_day`, `hottest_row`, `day_in_fahrenheit` are `snake_case`. |
|
||||
| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names are the lectures' (`temp_list`, `average_temp`, `data`, `rows`); `celsius` states the unit. |
|
||||
| 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. One suppression exists, `# noqa: E402` in `tests/test_analysis.py`, and it is explained: the import must follow `pytest.importorskip("pandas")` so that the module is skipped when the extra is missing. |
|
||||
| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | Every function is annotated, including the return types `pandas.DataFrame`, `list[int]`, `float` and `int`; the tests are annotated `-> None`. |
|
||||
| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | `day_in_fahrenheit` raises a specific `ValueError` with the day and the row count when the day is not in the table exactly once; nothing is caught or swallowed. |
|
||||
| 6 | No mutable default arguments and no shadowed builtins | Pass | The only default is an immutable `DATA_FILE`; no builtin is shadowed. |
|
||||
| 7 | Files, locks and connections are managed with context managers | N-A | The module opens no file itself; `pandas.read_csv` opens and closes the file. |
|
||||
| 8 | Public modules, classes and functions have docstrings that say what, not how | Pass | Module docstring and Doxygen `@brief`, `@param`, `@return` (and `@throws`) on every function; `doxygen Doxyfile` exits 0 with no warnings. |
|
||||
| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | N-A | The module neither logs nor prints. |
|
||||
| 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 lectures and to the tasks of [MIL-003]. |
|
||||
| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | `tests/test_analysis.py` has 13 tests named for behaviour, one for each Go/No-Go criterion of [MIL-003] plus the empty result for an unknown day; none depends on order or the network; `pytest` reports 27 passed. |
|
||||
| 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 8 source files. The one `Any` is `dict[Hashable, Any]` of `to_data_dict`, justified in a comment (the cell values of the table are heterogeneous) and given by the signature of `DataFrame.to_dict`. |
|
||||
| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | `pandas` and `pandas-stubs` are declared in the `pandas` and `dev` extras with lower bounds and are used; `dependencies = []` is unchanged and `csv_reader` still imports no `pandas`. |
|
||||
| 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 | Column names and Fahrenheit factors come from `constants.py`; the maximum is stated once (`max_temp`, reused by `hottest_row`); the temperature list is stated once (`to_temp_list`, reused by `average_temp`). |
|
||||
| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | `analysis` imports `pandas` and `weather.constants`; no import cycle; `csv_reader` does not import `analysis`. |
|
||||
| 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 | Small functions with one reason to change each; 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-003]: ../../milestones/mil-003-pandas-analysis.md
|
||||
[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
|
||||
[PP-001]: ../../project-plan.md
|
||||
[75d7e85]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/025-weather/commit/75d7e8525e2cdb149a00baab21d8f0f5f6b98686
|
||||
@@ -0,0 +1,113 @@
|
||||
## @file analysis.py
|
||||
# @brief Reads and analyses the weather data file with `pandas`.
|
||||
#
|
||||
# This is the second half of the lectures: the file is read in one line into a
|
||||
# `DataFrame`, converted to a dictionary and a list, and the average and
|
||||
# maximum temperature and the row selections are computed. This is the only
|
||||
# module of the package that imports `pandas`, which is an optional extra.
|
||||
|
||||
"""Reads and analyses the weather data file with pandas."""
|
||||
|
||||
from collections.abc import Hashable
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pandas
|
||||
|
||||
from weather.constants import (
|
||||
COLUMN_DAY,
|
||||
COLUMN_TEMP,
|
||||
DATA_FILE,
|
||||
FAHRENHEIT_DENOMINATOR,
|
||||
FAHRENHEIT_NUMERATOR,
|
||||
FAHRENHEIT_OFFSET,
|
||||
)
|
||||
|
||||
|
||||
def read_data(path: Path | str = DATA_FILE) -> pandas.DataFrame:
|
||||
"""! @brief Read a weather CSV file into a `DataFrame`.
|
||||
|
||||
@param path Path of the CSV file; the weather data file by default.
|
||||
@return The table, one row per day, with the columns of the file.
|
||||
"""
|
||||
return pandas.read_csv(path)
|
||||
|
||||
|
||||
def to_data_dict(data: pandas.DataFrame) -> dict[Hashable, Any]: # Any: cell values
|
||||
"""! @brief Convert the table to a dictionary of columns.
|
||||
|
||||
@param data The weather table.
|
||||
@return A dictionary that maps each column name to its values by row index.
|
||||
"""
|
||||
return data.to_dict()
|
||||
|
||||
|
||||
def to_temp_list(data: pandas.DataFrame) -> list[int]:
|
||||
"""! @brief Convert the temperature column to a list.
|
||||
|
||||
@param data The weather table.
|
||||
@return The temperatures in row order.
|
||||
"""
|
||||
return data[COLUMN_TEMP].to_list()
|
||||
|
||||
|
||||
def average_temp(data: pandas.DataFrame) -> float:
|
||||
"""! @brief Compute the average temperature from the sum and the length.
|
||||
|
||||
@param data The weather table.
|
||||
@return The average temperature.
|
||||
"""
|
||||
temp_list = to_temp_list(data)
|
||||
return sum(temp_list) / len(temp_list)
|
||||
|
||||
|
||||
def average_temp_mean(data: pandas.DataFrame) -> float:
|
||||
"""! @brief Compute the average temperature with `Series.mean()`.
|
||||
|
||||
@param data The weather table.
|
||||
@return The average temperature.
|
||||
"""
|
||||
return float(data[COLUMN_TEMP].mean())
|
||||
|
||||
|
||||
def max_temp(data: pandas.DataFrame) -> int:
|
||||
"""! @brief Find the highest temperature.
|
||||
|
||||
@param data The weather table.
|
||||
@return The maximum of the temperature column.
|
||||
"""
|
||||
return int(data[COLUMN_TEMP].max())
|
||||
|
||||
|
||||
def row_for_day(data: pandas.DataFrame, day: str) -> pandas.DataFrame:
|
||||
"""! @brief Select the rows of one day.
|
||||
|
||||
@param data The weather table.
|
||||
@param day Name of the day, such as `Monday`.
|
||||
@return The matching rows; empty when the day is not in the table.
|
||||
"""
|
||||
return data.loc[data[COLUMN_DAY] == day]
|
||||
|
||||
|
||||
def hottest_row(data: pandas.DataFrame) -> pandas.DataFrame:
|
||||
"""! @brief Select the row or rows with the highest temperature.
|
||||
|
||||
@param data The weather table.
|
||||
@return The rows whose temperature equals the maximum.
|
||||
"""
|
||||
return data.loc[data[COLUMN_TEMP] == max_temp(data)]
|
||||
|
||||
|
||||
def day_in_fahrenheit(data: pandas.DataFrame, day: str) -> float:
|
||||
"""! @brief Convert the temperature of one day from Celsius to Fahrenheit.
|
||||
|
||||
@param data The weather table.
|
||||
@param day Name of the day, such as `Monday`.
|
||||
@return The temperature in degrees Fahrenheit.
|
||||
@throws ValueError When the day is not in the table exactly once.
|
||||
"""
|
||||
rows = row_for_day(data, day)
|
||||
if len(rows) != 1:
|
||||
raise ValueError(f"Expected exactly one row for day {day!r}, got {len(rows)}")
|
||||
celsius = int(rows[COLUMN_TEMP].iloc[0])
|
||||
return celsius * FAHRENHEIT_NUMERATOR / FAHRENHEIT_DENOMINATOR + FAHRENHEIT_OFFSET
|
||||
@@ -0,0 +1,106 @@
|
||||
"""Tests of the pandas analysis against the real data file and small files."""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
pandas = pytest.importorskip("pandas")
|
||||
|
||||
from weather import analysis, csv_reader # noqa: E402
|
||||
|
||||
LECTURE_TEMPERATURES = [12, 14, 15, 14, 21, 22, 24]
|
||||
COLUMNS = ["day", "temp", "condition"]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def small_csv(tmp_path: Path) -> Path:
|
||||
file = tmp_path / "small.csv"
|
||||
file.write_text("day,temp,condition\nMonday,3,Rain\nTuesday,-4,Snow\n")
|
||||
return file
|
||||
|
||||
|
||||
def test_read_data_gives_a_dataframe_with_seven_rows_and_three_columns() -> None:
|
||||
data = analysis.read_data()
|
||||
|
||||
assert isinstance(data, pandas.DataFrame)
|
||||
assert len(data) == 7
|
||||
assert list(data.columns) == COLUMNS
|
||||
|
||||
|
||||
def test_a_column_of_the_data_is_a_series() -> None:
|
||||
data = analysis.read_data()
|
||||
|
||||
assert isinstance(data["temp"], pandas.Series)
|
||||
|
||||
|
||||
def test_to_data_dict_has_a_key_for_each_column() -> None:
|
||||
data_dict = analysis.to_data_dict(analysis.read_data())
|
||||
|
||||
assert list(data_dict) == COLUMNS
|
||||
|
||||
|
||||
def test_to_temp_list_gives_the_lecture_temperatures() -> None:
|
||||
assert analysis.to_temp_list(analysis.read_data()) == LECTURE_TEMPERATURES
|
||||
|
||||
|
||||
def test_both_averages_agree_and_equal_the_sum_over_the_count() -> None:
|
||||
data = analysis.read_data()
|
||||
expected = 122 / 7
|
||||
|
||||
assert analysis.average_temp(data) == pytest.approx(expected)
|
||||
assert analysis.average_temp_mean(data) == pytest.approx(expected)
|
||||
mean_way = analysis.average_temp_mean(data)
|
||||
assert analysis.average_temp(data) == pytest.approx(mean_way)
|
||||
|
||||
|
||||
def test_max_temp_gives_the_highest_temperature() -> None:
|
||||
assert analysis.max_temp(analysis.read_data()) == 24
|
||||
|
||||
|
||||
def test_row_for_day_gives_the_single_row_of_monday() -> None:
|
||||
rows = analysis.row_for_day(analysis.read_data(), "Monday")
|
||||
|
||||
assert len(rows) == 1
|
||||
assert rows["condition"].iloc[0] == "Sunny"
|
||||
assert rows["temp"].iloc[0] == 12
|
||||
|
||||
|
||||
def test_row_for_day_gives_an_empty_result_for_an_unknown_day() -> None:
|
||||
assert analysis.row_for_day(analysis.read_data(), "Funday").empty
|
||||
|
||||
|
||||
def test_hottest_row_is_the_row_of_sunday() -> None:
|
||||
rows = analysis.hottest_row(analysis.read_data())
|
||||
|
||||
assert len(rows) == 1
|
||||
assert rows["day"].iloc[0] == "Sunday"
|
||||
assert rows["temp"].iloc[0] == 24
|
||||
|
||||
|
||||
def test_day_in_fahrenheit_converts_monday() -> None:
|
||||
assert analysis.day_in_fahrenheit(analysis.read_data(), "Monday") == pytest.approx(
|
||||
53.6
|
||||
)
|
||||
|
||||
|
||||
def test_day_in_fahrenheit_raises_value_error_for_an_unknown_day() -> None:
|
||||
with pytest.raises(ValueError, match="Funday"):
|
||||
analysis.day_in_fahrenheit(analysis.read_data(), "Funday")
|
||||
|
||||
|
||||
def test_csv_and_pandas_readers_agree_on_the_temperatures() -> None:
|
||||
pandas_temps = analysis.to_temp_list(analysis.read_data())
|
||||
|
||||
assert pandas_temps == csv_reader.read_temperatures()
|
||||
|
||||
|
||||
def test_read_data_reads_a_small_file_through_the_path_argument(
|
||||
small_csv: Path,
|
||||
) -> None:
|
||||
for path in (small_csv, str(small_csv)):
|
||||
data = analysis.read_data(path)
|
||||
|
||||
assert len(data) == 2
|
||||
assert analysis.to_temp_list(data) == [3, -4]
|
||||
assert analysis.max_temp(data) == 3
|
||||
assert analysis.day_in_fahrenheit(data, "Tuesday") == pytest.approx(24.8)
|
||||
Reference in New Issue
Block a user