diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index c0e749e..eeb680f 100644 --- a/docs/artifact-registry.md +++ b/docs/artifact-registry.md @@ -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 diff --git a/docs/sqa/reviews/rc-011-mil-003-code.md b/docs/sqa/reviews/rc-011-mil-003-code.md new file mode 100644 index 0000000..bb3463e --- /dev/null +++ b/docs/sqa/reviews/rc-011-mil-003-code.md @@ -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 diff --git a/src/weather/analysis.py b/src/weather/analysis.py new file mode 100644 index 0000000..579aaff --- /dev/null +++ b/src/weather/analysis.py @@ -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 diff --git a/tests/test_analysis.py b/tests/test_analysis.py new file mode 100644 index 0000000..4c41580 --- /dev/null +++ b/tests/test_analysis.py @@ -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)