From 9c1484c53ea4c18b888778821a919e014b72d388 Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Tue, 16 Jun 2026 12:40:31 -0400 Subject: [PATCH 1/3] feat: Add RecordModel base and rename readers/writers to Model* Extract a mixin-free RecordModel base out of Metric (closes #10), and rename MetricReader/MetricWriter to ModelReader/ModelWriter so the IO classes operate on any RecordModel. Metric is behaviorally unchanged: it subclasses RecordModel plus the same converter mixins in the same MRO order. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 16 +- README.md | 16 +- fgmetric/__init__.py | 10 +- fgmetric/converters/_counter_pivot_table.py | 44 +++++- fgmetric/metric.py | 137 ++---------------- .../{metric_reader.py => model_reader.py} | 32 ++-- .../{metric_writer.py => model_writer.py} | 50 +++---- fgmetric/record_model.py | 130 +++++++++++++++++ tests/benchmarks/test_benchmark_runtime.py | 4 +- tests/test_collections.py | 44 +++--- tests/test_compression.py | 34 ++--- tests/test_metric.py | 4 +- ..._metric_reader.py => test_model_reader.py} | 108 +++++++------- ..._metric_writer.py => test_model_writer.py} | 82 +++++------ tests/test_record_model.py | 84 +++++++++++ tests/test_roundtrip.py | 16 +- 16 files changed, 479 insertions(+), 332 deletions(-) rename fgmetric/{metric_reader.py => model_reader.py} (83%) rename fgmetric/{metric_writer.py => model_writer.py} (72%) create mode 100644 fgmetric/record_model.py rename tests/{test_metric_reader.py => test_model_reader.py} (82%) rename tests/{test_metric_writer.py => test_model_writer.py} (73%) create mode 100644 tests/test_record_model.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 514c637..40fe249 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,17 +6,19 @@ All notable changes to this project will be documented in this file. ### Added -- Introduce `MetricReader` for iterating metrics from any text-IO source (#41) -- Add an `encoding` kwarg on `MetricReader.open` and `MetricWriter.open` (#43) +- Add `RecordModel`, a mixin-free base class providing the default tabular read/write surface without the `fgmetric.converters` mixins; `Metric` now subclasses it (#10) +- Introduce `ModelReader` for iterating records from any text-IO source (#41) +- Add an `encoding` kwarg on `ModelReader.open` and `ModelWriter.open` (#43) - Transparent gzip/bz2/xz support via `xopen` (#44) -- Infer the delimiter from the file extension in `MetricReader.open`/`MetricWriter.open`; pass `delimiter=` to override (#61) +- Infer the delimiter from the file extension in `ModelReader.open`/`ModelWriter.open`; pass `delimiter=` to override (#61) ### Changed -- `MetricWriter` is IO-first; open a path with `MetricWriter.open()` rather than passing it to the constructor (#42) -- `Metric.read` is now a thin wrapper over `MetricReader.open` and reads eagerly, returning a `list` instead of a lazy generator; it accepts `str` paths in addition to `Path` and gains transparent decompression and the `encoding` kwarg. Use `MetricReader.open` to stream metrics without holding them all in memory (#62) -- `MetricReader.open` and `MetricWriter.open` eagerly validate that the path is readable/writable on context entry (#66) -- `MetricWriter.open` now refuses to overwrite an existing file by default, raising `FileExistsError`; pass `overwrite=True` to truncate and overwrite it (#69) +- Rename `MetricReader`/`MetricWriter` to `ModelReader`/`ModelWriter`; the IO classes now read and write any `RecordModel` subclass, not just `Metric` (#10) +- `ModelWriter` is IO-first; open a path with `ModelWriter.open()` rather than passing it to the constructor (#42) +- `Metric.read` is now a thin wrapper over `ModelReader.open` and reads eagerly, returning a `list` instead of a lazy generator; it accepts `str` paths in addition to `Path` and gains transparent decompression and the `encoding` kwarg. Use `ModelReader.open` to stream metrics without holding them all in memory (#62) +- `ModelReader.open` and `ModelWriter.open` eagerly validate that the path is readable/writable on context entry (#66) +- `ModelWriter.open` now refuses to overwrite an existing file by default, raising `FileExistsError`; pass `overwrite=True` to truncate and overwrite it (#69) ## [0.3.0] - 2026-05-12 diff --git a/README.md b/README.md index 32d4ed3..b5ff2ad 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ for metric in AlignmentMetric.read(path): Define a class to represent each row: ```python -from fgmetric import Metric, MetricReader, MetricWriter +from fgmetric import Metric, ModelReader, ModelWriter class AlignmentMetric(Metric): @@ -100,25 +100,25 @@ metrics = [ AlignmentMetric(read_name="read1", mapping_quality=60), AlignmentMetric(read_name="read2", mapping_quality=30, is_duplicate=True), ] -with MetricWriter.open(AlignmentMetric, "output.tsv") as writer: +with ModelWriter.open(AlignmentMetric, "output.tsv") as writer: writer.writeall(metrics) ``` -`Metric.read()` reads the whole file into a list. To stream metrics one at a time without holding them all in memory, use `MetricReader.open()`: +`Metric.read()` reads the whole file into a list. To stream metrics one at a time without holding them all in memory, use `ModelReader.open()`: ```python # Streaming from a path -with MetricReader.open(AlignmentMetric, "alignments.tsv") as reader: +with ModelReader.open(AlignmentMetric, "alignments.tsv") as reader: for metric in reader: print(f"{metric.read_name}: MQ={metric.mapping_quality}") ``` -To read from an open file handle or any other text IO source (e.g. `StringIO`), use `MetricReader` directly: +To read from an open file handle or any other text IO source (e.g. `StringIO`), use `ModelReader` directly: ```python # Reading from an IO source with open("alignments.tsv") as handle: - reader = MetricReader(AlignmentMetric, handle) + reader = ModelReader(AlignmentMetric, handle) for metric in reader: print(f"{metric.read_name}: MQ={metric.mapping_quality}") ``` @@ -149,7 +149,7 @@ Pass `delimiter=` to override inference, or to read or write a file whose extens ```python # Pipe-delimited file with an unrecognized extension -with MetricWriter.open(MyMetric, "output.dat", delimiter="|") as writer: +with ModelWriter.open(MyMetric, "output.dat", delimiter="|") as writer: ... ``` @@ -161,7 +161,7 @@ Reading and writing transparently handle gzip, bzip2, and xz files based on the for metric in AlignmentMetric.read("alignments.tsv.gz"): ... -with MetricWriter.open(AlignmentMetric, "output.tsv.bz2") as writer: +with ModelWriter.open(AlignmentMetric, "output.tsv.bz2") as writer: writer.writeall(metrics) ``` diff --git a/fgmetric/__init__.py b/fgmetric/__init__.py index 30beebc..a404281 100644 --- a/fgmetric/__init__.py +++ b/fgmetric/__init__.py @@ -1,9 +1,11 @@ from fgmetric.metric import Metric -from fgmetric.metric_reader import MetricReader -from fgmetric.metric_writer import MetricWriter +from fgmetric.model_reader import ModelReader +from fgmetric.model_writer import ModelWriter +from fgmetric.record_model import RecordModel __all__ = [ "Metric", - "MetricReader", - "MetricWriter", + "ModelReader", + "ModelWriter", + "RecordModel", ] diff --git a/fgmetric/converters/_counter_pivot_table.py b/fgmetric/converters/_counter_pivot_table.py index 96b26f5..1147dfd 100644 --- a/fgmetric/converters/_counter_pivot_table.py +++ b/fgmetric/converters/_counter_pivot_table.py @@ -4,7 +4,6 @@ from typing import final from typing import get_args -from pydantic import BaseModel from pydantic import SerializationInfo from pydantic import SerializerFunctionWrapHandler from pydantic import model_serializer @@ -13,9 +12,14 @@ from fgmetric._typing_extensions import is_counter from fgmetric._typing_extensions import is_optional +from fgmetric.record_model import RecordModel -class CounterPivotTable(BaseModel): +# NB: This mixin inherits `RecordModel` rather than `BaseModel` (unlike the other converters) so +# that it can override `RecordModel._header_fieldnames` and call `super()` to reuse the default +# alias-aware header. It is the only converter that rewrites the on-disk column set, so it is the +# only one that needs the base header contract in scope. +class CounterPivotTable(RecordModel): """ A mixin to support pivot table representations of Counters. @@ -201,6 +205,42 @@ def _get_counter_enum(cls) -> type[StrEnum] | None: # noqa: C901 return enum_cls + @final + @classmethod + def _header_fieldnames(cls) -> list[str]: + """ + Return the header fieldnames with the Counter field pivoted into per-member columns. + + Overrides `RecordModel._header_fieldnames` to match this mixin's wide serialization: the + single `Counter[T]` field has no on-disk column of its own, so it is dropped from the + default header and replaced by one column per enum member (in enum declaration order), + matching the output of `_pivot_counter_values`. + + Returns: + The list of fieldnames to use as the header row. + + Example: + Given a model with ``name: str`` and ``counts: Counter[Color]`` where + ``Color`` has members ``RED``, ``GREEN``, ``BLUE``: + + ```python + cls._header_fieldnames() + # -> ["name", "red", "green", "blue"] + ``` + """ + # The Counter field has no alias (aliases are rejected for Counter fields), so its + # serialized name equals its field name; drop it from the default header. + fieldnames = [ + name for name in super()._header_fieldnames() if name != cls._counter_fieldname + ] + + if cls._counter_enum is None: + # Short circuit if we don't have a Counter field + return fieldnames + + # Replace the dropped Counter field with one column per enum member + return fieldnames + [member.value for member in cls._counter_enum] + @final @model_validator(mode="before") @classmethod diff --git a/fgmetric/metric.py b/fgmetric/metric.py index 37a0712..121ee21 100644 --- a/fgmetric/metric.py +++ b/fgmetric/metric.py @@ -1,43 +1,41 @@ -from abc import ABC -from collections.abc import Sequence -from pathlib import Path from typing import ClassVar -from typing import Self - -from pydantic import BaseModel from fgmetric.converters import CounterPivotTable from fgmetric.converters import DelimitedList from fgmetric.converters import NullSentinels -from fgmetric.metric_reader import MetricReader +from fgmetric.record_model import RecordModel class Metric( DelimitedList, CounterPivotTable, NullSentinels, - BaseModel, - ABC, + RecordModel, ): """ Abstract base class for defining structured "metric" data models. - This class combines Pydantic's `BaseModel` with `ABC` to provide a foundation for creating - type-safe metric classes that can be easily read from and written to delimited text files (e.g., - TSV, CSV). It leverages Pydantic's automatic validation and type conversion while providing - convenient class methods for parsing metrics from files. + `Metric` is a `RecordModel` with the default `fgmetric.converters` mixins layered on. It + provides the same file-reading and -writing surface as `RecordModel` (see `RecordModel.read`, + `ModelReader`, and `ModelWriter`) plus the conversion behaviors below. Subclasses should define + their fields using Pydantic field annotations. Metrics are delimited files containing a header and zero or more rows for metric values. When using a `Metric` to read a delimited file, the `Metric`'s fields correspond to the columns and - header of the file. Subclasses should define their fields using Pydantic field annotations. + header of the file. + + Relative to a plain `RecordModel`, `Metric` adds the following custom + serialization/deserialization behaviors: - `Metric` includes the following custom serialization/deserialization behaviors: 1. **Null sentinels.** Empty strings in Optional fields are converted to `None` before field validation. The sentinel values converted to `None` can be overridden by the `null_sentinels` class variable. See `NullSentinels`. 2. **Delimited lists.** Any field typed as `list[T]` will be parsed from and serialized to a delimited string. The list delimiter may be controlled by the `collection_delimiter` class variable. + 3. **Counter pivot tables.** A single field typed as `Counter[T]` (where `T` is a `StrEnum`) + is pivoted into one column per enum member during serialization and folded back during + validation. Class Variables: collection_delimiter: A single-character delimiter used to split and join `list` fields @@ -59,112 +57,3 @@ class AlignmentMetric(Metric): """ null_sentinels: ClassVar[frozenset[str]] = frozenset({""}) - - @classmethod - def read( - cls, - path: Path | str, - # NB: these defaults mirror `MetricReader.open()`; keep them in sync. - delimiter: str = "\t", - fieldnames: Sequence[str] | None = None, - encoding: str = "utf-8-sig", - ) -> list[Self]: - """ - Read all Metric instances from a file path. - - Eager wrapper around `MetricReader.open()`: the file is opened, parsed, and closed - before this method returns, collecting every row into a list. Because reading happens - up front, IO and validation errors surface here at the call site rather than partway - through iteration. - - See `MetricReader.open()` for the file-handling behavior shared by both APIs - encoding, - automatic decompression of `.gz`/`.bz2`/`.xz` files, and how `fieldnames` selects - header vs. headerless parsing. To stream a large file without holding every row in - memory, or to read from an already-open handle or other text IO source, use - `MetricReader` directly. - - Args: - path: Filesystem path to the input file. - delimiter: The input file delimiter. - fieldnames: Optional sequence of field names. If provided, the input is treated as - headerless and these names are used as the column headers. - encoding: The text encoding used to decode the file. - - Returns: - A list of instances of the calling Metric subclass, one per data row. - - Raises: - FileNotFoundError: If `path` does not exist. - LookupError: If `encoding` is not a known codec. - UnicodeDecodeError: If the file's bytes cannot be decoded using `encoding`. - ValueError: If `fieldnames` is supplied and the first row matches it (a likely - forgotten header). - ValidationError: If a row fails `Metric` validation, e.g. a missing required field - or a value of the wrong type. - - Example: - `read` is eager and returns a list, so the whole file is available at once: - - ```python - metrics = AlignmentMetric.read("metrics.txt") - print(f"read {len(metrics)} rows") - for m in metrics: - print(m.read_name, m.mapping_quality) - ``` - """ - with MetricReader.open( - cls, - path, - delimiter=delimiter, - fieldnames=fieldnames, - encoding=encoding, - ) as reader: - return list(reader) - - @classmethod - def _header_fieldnames(cls) -> list[str]: - """ - Return the fieldnames to use as a header row when writing metrics to a file. - - This method is used by `MetricWriter` to construct the underlying `csv.DictWriter`. - It returns the fieldnames that will appear in serialized output, which may differ from - the model's field names when aliases are used. - - Note: - This method is deliberately not used during reading/validation; see - `MetricReader` for the headerless-input path. - - Returns: - The list of fieldnames to use as the header row. - - Example: - Given a model with ``name: str`` and ``counts: Counter[Color]`` where - ``Color`` has members ``RED``, ``GREEN``, ``BLUE``: - - ```python - cls._header_fieldnames() - # -> ["name", "red", "green", "blue"] - ``` - """ - # TODO: support returning the set of fields that would be constructed if the class has a - # custom model serializer - - # Resolve each field to the key it serializes under (its alias, when one is set), so the - # header matches the keys produced by `model_dump(by_alias=True)`. The Counter field, if - # present, is dropped here and replaced by its pivoted enum-member columns below. - fieldnames: list[str] - fieldnames = [ - info.serialization_alias or info.alias or name - for name, info in cls.model_fields.items() - if name != cls._counter_fieldname - ] - - if cls._counter_fieldname is None: - # Short circuit if we don't have a Counter field - return fieldnames - - # Add the enum members - assert cls._counter_enum is not None # type narrowing - fieldnames += [member.value for member in cls._counter_enum] - - return fieldnames diff --git a/fgmetric/metric_reader.py b/fgmetric/model_reader.py similarity index 83% rename from fgmetric/metric_reader.py rename to fgmetric/model_reader.py index e807074..b806955 100644 --- a/fgmetric/metric_reader.py +++ b/fgmetric/model_reader.py @@ -14,12 +14,12 @@ from fgmetric._paths import path_read_error if TYPE_CHECKING: - from fgmetric.metric import Metric + from fgmetric.record_model import RecordModel -class MetricReader[T: Metric]: +class ModelReader[T: RecordModel]: """ - Iterate `Metric` instances from a text IO source. + Iterate `RecordModel` instances from a text IO source. Constructed with any iterable of strings (file handle, StringIO, list of lines). The reader does not own the source; callers manage its lifecycle. @@ -27,21 +27,21 @@ class MetricReader[T: Metric]: construction, `open` owns the file it opens and closes it on context exit. """ - _metric_class: type[T] + _model_class: type[T] _records: Iterator[dict[str, str | None]] def __init__( self, - metric_class: type[T], + model_class: type[T], source: Iterable[str], delimiter: str = "\t", fieldnames: Sequence[str] | None = None, ) -> None: """ - Initialize a new `MetricReader`. + Initialize a new `ModelReader`. Args: - metric_class: Metric class. + model_class: The `RecordModel` subclass to validate each row into. source: An iterable of strings (e.g., file handle, StringIO) to read from. delimiter: The input file delimiter. fieldnames: Optional sequence of field names. If provided, the input is treated as @@ -51,7 +51,7 @@ def __init__( ValueError: If `fieldnames` is supplied and the first row appears to be a header that matches it. """ - self._metric_class = metric_class + self._model_class = model_class records: Iterator[dict[str, str | None]] = DictReader( source, fieldnames=fieldnames, @@ -74,17 +74,17 @@ def __init__( @contextmanager def open( cls, - metric_class: type[T], + model_class: type[T], path: Path | str, delimiter: str | None = None, fieldnames: Sequence[str] | None = None, encoding: str = "utf-8-sig", ) -> Iterator[Self]: """ - Open `path` and yield a `MetricReader` over its contents. + Open `path` and yield a `ModelReader` over its contents. This is a context manager: bind it in a `with` statement and iterate the reader it - yields. `reader = MetricReader.open(...)` without `with` binds the context manager + yields. `reader = ModelReader.open(...)` without `with` binds the context manager itself, not a reader, so it will not iterate. The file is opened with the given encoding and closed on context exit. The default encoding, @@ -94,7 +94,7 @@ def open( files are transparently decompressed. Args: - metric_class: Metric class. + model_class: The `RecordModel` subclass to validate each row into. path: Filesystem path to the input file. delimiter: The input file delimiter. When `None` (the default), the delimiter is inferred from the file extension: `.csv` → comma; `.tsv`, `.txt`, `.tab`, or @@ -106,7 +106,7 @@ def open( encoding: The text encoding used to decode the file. Yields: - A `MetricReader` over the opened file. + A `ModelReader` over the opened file. Raises: FileNotFoundError: If `path` does not exist. @@ -117,7 +117,7 @@ def open( Example: ```python - with MetricReader.open(AlignmentMetric, "metrics.txt") as reader: + with ModelReader.open(AlignmentMetric, "metrics.txt") as reader: for metric in reader: ... ``` @@ -127,10 +127,10 @@ def open( if delimiter is None: delimiter = infer_delimiter(path) with xopen(path, mode="rt", encoding=encoding) as handle: - yield cls(metric_class, handle, delimiter, fieldnames) + yield cls(model_class, handle, delimiter, fieldnames) def __iter__(self) -> Self: return self def __next__(self) -> T: - return self._metric_class.model_validate(next(self._records)) + return self._model_class.model_validate(next(self._records)) diff --git a/fgmetric/metric_writer.py b/fgmetric/model_writer.py similarity index 72% rename from fgmetric/metric_writer.py rename to fgmetric/model_writer.py index 98ef4a0..9786d58 100644 --- a/fgmetric/metric_writer.py +++ b/fgmetric/model_writer.py @@ -10,12 +10,12 @@ from fgmetric._delimiter import infer_delimiter from fgmetric._paths import path_write_error -from fgmetric.metric import Metric +from fgmetric.record_model import RecordModel -class MetricWriter[T: Metric]: +class ModelWriter[T: RecordModel]: """ - Write `Metric` instances to a text IO sink. + Write `RecordModel` instances to a text IO sink. Construction takes a writable text IO and writes the header row immediately. The writer does not own the sink; callers manage its lifecycle. Use the @@ -23,29 +23,29 @@ class MetricWriter[T: Metric]: construction, `open` owns the file it opens and closes it on context exit. """ - _metric_class: type[T] + _model_class: type[T] _writer: DictWriter[str] def __init__( self, - metric_class: type[T], + model_class: type[T], sink: TextIO, delimiter: str = "\t", lineterminator: str = "\n", ) -> None: """ - Initialize a new `MetricWriter` and write the header row to `sink`. + Initialize a new `ModelWriter` and write the header row to `sink`. Args: - metric_class: Metric class. + model_class: The `RecordModel` subclass whose instances will be written. sink: Writable text IO (e.g., file handle, StringIO) to write to. delimiter: The output file delimiter. lineterminator: The string used to terminate lines. """ - self._metric_class = metric_class + self._model_class = model_class self._writer = DictWriter( f=sink, - fieldnames=metric_class._header_fieldnames(), + fieldnames=model_class._header_fieldnames(), delimiter=delimiter, lineterminator=lineterminator, ) @@ -55,7 +55,7 @@ def __init__( @contextmanager def open( cls, - metric_class: type[T], + model_class: type[T], path: Path | str, delimiter: str | None = None, lineterminator: str = "\n", @@ -63,10 +63,10 @@ def open( overwrite: bool = False, ) -> Iterator[Self]: """ - Open `path` for writing and yield a `MetricWriter` over it. + Open `path` for writing and yield a `ModelWriter` over it. This is a context manager: bind it in a `with` statement and write through the writer - it yields. `writer = MetricWriter.open(...)` without `with` binds the context manager + it yields. `writer = ModelWriter.open(...)` without `with` binds the context manager itself, not a writer. Compression is selected automatically based on the output file extension: plaintext, gzip @@ -75,7 +75,7 @@ def open( The header is written on context entry; the file is closed on context exit. Args: - metric_class: Metric class. + model_class: The `RecordModel` subclass whose instances will be written. path: Filesystem path to the output file. delimiter: The output file delimiter. When `None` (the default), the delimiter is inferred from the file extension: `.csv` → comma; `.tsv`, `.txt`, `.tab`, or @@ -87,7 +87,7 @@ def open( `FileExistsError`. Pass `True` to destructively overwrite an existing file. Yields: - A `MetricWriter` over the opened file. + A `ModelWriter` over the opened file. Raises: FileNotFoundError: If the parent directory of `path` does not exist. @@ -101,7 +101,7 @@ def open( Example: ```python - with MetricWriter.open(AlignmentMetric, "metrics.txt") as writer: + with ModelWriter.open(AlignmentMetric, "metrics.txt") as writer: writer.writeall(metrics) ``` """ @@ -110,23 +110,23 @@ def open( if delimiter is None: delimiter = infer_delimiter(path) with xopen(path, mode="wt", encoding=encoding) as handle: - yield cls(metric_class, handle, delimiter, lineterminator) + yield cls(model_class, handle, delimiter, lineterminator) - def write(self, metric: T) -> None: + def write(self, model: T) -> None: """ - Write a single `Metric` instance. + Write a single `RecordModel` instance. Args: - metric: An instance of the writer's metric class. + model: An instance of the writer's model class. """ - self._writer.writerow(metric.model_dump(mode="json", by_alias=True)) + self._writer.writerow(model.model_dump(mode="json", by_alias=True)) - def writeall(self, metrics: Iterable[T]) -> None: + def writeall(self, models: Iterable[T]) -> None: """ - Write multiple `Metric` instances. + Write multiple `RecordModel` instances. Args: - metrics: An iterable of instances of the writer's metric class. + models: An iterable of instances of the writer's model class. """ - for metric in metrics: - self.write(metric) + for model in models: + self.write(model) diff --git a/fgmetric/record_model.py b/fgmetric/record_model.py new file mode 100644 index 0000000..278fa28 --- /dev/null +++ b/fgmetric/record_model.py @@ -0,0 +1,130 @@ +from abc import ABC +from collections.abc import Sequence +from pathlib import Path +from typing import Self + +from pydantic import BaseModel + +from fgmetric.model_reader import ModelReader + + +class RecordModel(BaseModel, ABC): + """ + Abstract base class for record-oriented data models read from and written to delimited files. + + `RecordModel` combines Pydantic's `BaseModel` with `ABC` to provide a foundation for type-safe + models that map one-to-one onto the rows of a delimited text file (e.g., TSV, CSV). It performs + the default tabular parsing only: each model field corresponds to a column, with Pydantic + handling validation and type conversion. It carries none of the optional conversion behaviors + (delimited lists, counter pivot tables, null sentinels) layered on by `Metric` and the + `fgmetric.converters` mixins. + + Use `RecordModel` directly when you want plain field-per-column parsing without the mixin + machinery; use `Metric` when you want the batteries-included behaviors. `ModelReader` and + `ModelWriter` operate on any `RecordModel` subclass. + + Subclasses should define their fields using Pydantic field annotations. + + Example: + ```python + class AlignmentRecord(RecordModel): + read_name: str + mapping_quality: int + is_duplicate: bool = False + + # Read records from a TSV file + for record in AlignmentRecord.read("records.txt"): + print(record.read_name, record.mapping_quality) + ``` + """ + + @classmethod + def read( + cls, + path: Path | str, + # NB: these defaults mirror `ModelReader.open()`; keep them in sync. + delimiter: str = "\t", + fieldnames: Sequence[str] | None = None, + encoding: str = "utf-8-sig", + ) -> list[Self]: + """ + Read all instances from a file path. + + Eager wrapper around `ModelReader.open()`: the file is opened, parsed, and closed + before this method returns, collecting every row into a list. Because reading happens + up front, IO and validation errors surface here at the call site rather than partway + through iteration. + + See `ModelReader.open()` for the file-handling behavior shared by both APIs - encoding, + automatic decompression of `.gz`/`.bz2`/`.xz` files, and how `fieldnames` selects + header vs. headerless parsing. To stream a large file without holding every row in + memory, or to read from an already-open handle or other text IO source, use + `ModelReader` directly. + + Args: + path: Filesystem path to the input file. + delimiter: The input file delimiter. + fieldnames: Optional sequence of field names. If provided, the input is treated as + headerless and these names are used as the column headers. + encoding: The text encoding used to decode the file. + + Returns: + A list of instances of the calling subclass, one per data row. + + Raises: + FileNotFoundError: If `path` does not exist. + LookupError: If `encoding` is not a known codec. + UnicodeDecodeError: If the file's bytes cannot be decoded using `encoding`. + ValueError: If `fieldnames` is supplied and the first row matches it (a likely + forgotten header). + ValidationError: If a row fails validation, e.g. a missing required field + or a value of the wrong type. + + Example: + `read` is eager and returns a list, so the whole file is available at once: + + ```python + records = AlignmentRecord.read("records.txt") + print(f"read {len(records)} rows") + for r in records: + print(r.read_name, r.mapping_quality) + ``` + """ + with ModelReader.open( + cls, + path, + delimiter=delimiter, + fieldnames=fieldnames, + encoding=encoding, + ) as reader: + return list(reader) + + @classmethod + def _header_fieldnames(cls) -> list[str]: + """ + Return the fieldnames to use as a header row when writing records to a file. + + This method is used by `ModelWriter` to construct the underlying `csv.DictWriter`. + It returns the fieldnames that will appear in serialized output, which may differ from + the model's field names when aliases are used. + + Subclasses and mixins (e.g., `CounterPivotTable`) may override this method to adjust the + header to match a custom serializer. Such overrides should call `super()` to obtain the + default field-per-column header and then transform it. + + Note: + This method is deliberately not used during reading/validation; see + `ModelReader` for the headerless-input path. + + Returns: + The list of fieldnames to use as the header row. + """ + # TODO: support returning the set of fields that would be constructed if the class has a + # custom model serializer + + # Resolve each field to the key it serializes under (its alias, when one is set), so the + # header matches the keys produced by `model_dump(by_alias=True)`. + return [ + info.serialization_alias or info.alias or name + for name, info in cls.model_fields.items() + ] diff --git a/tests/benchmarks/test_benchmark_runtime.py b/tests/benchmarks/test_benchmark_runtime.py index ee1fc4d..53c2b2e 100644 --- a/tests/benchmarks/test_benchmark_runtime.py +++ b/tests/benchmarks/test_benchmark_runtime.py @@ -17,7 +17,7 @@ @pytest.mark.parametrize("num_rows", NUM_ROWS) def test_fgmetric(benchmark: BenchmarkFixture, benchmark_data: Path, num_rows: str) -> None: from fgmetric import Metric - from fgmetric import MetricReader + from fgmetric import ModelReader class DemoMetric(Metric): foo: int @@ -30,7 +30,7 @@ class DemoMetric(Metric): tsv = benchmark_data / f"demo.{num_rows}.tsv" def read_all() -> list[DemoMetric]: - with MetricReader.open(DemoMetric, tsv) as reader: + with ModelReader.open(DemoMetric, tsv) as reader: return list(reader) benchmark(read_all) diff --git a/tests/test_collections.py b/tests/test_collections.py index 8f3c57e..b572572 100644 --- a/tests/test_collections.py +++ b/tests/test_collections.py @@ -9,8 +9,8 @@ from pydantic import PlainSerializer from fgmetric import Metric -from fgmetric import MetricReader -from fgmetric import MetricWriter +from fgmetric import ModelReader +from fgmetric import ModelWriter def test_comma_delimited_list(tmp_path: Path) -> None: @@ -30,7 +30,7 @@ class FakeMetric(Metric): fout.write("Nils\t1,2,3\n") fout.write("Tim\t\n") - with MetricReader.open(FakeMetric, fpath_to_read) as reader: + with ModelReader.open(FakeMetric, fpath_to_read) as reader: metrics = list(reader) assert len(metrics) == 2 @@ -41,8 +41,8 @@ class FakeMetric(Metric): # Test writing fpath_to_write = tmp_path / "written.txt" - writer: MetricWriter[FakeMetric] - with MetricWriter.open(FakeMetric, fpath_to_write) as writer: + writer: ModelWriter[FakeMetric] + with ModelWriter.open(FakeMetric, fpath_to_write) as writer: writer.writeall(metrics) with fpath_to_write.open("r") as f: @@ -68,7 +68,7 @@ class FakeMetric(Metric): fout.write("name\tvalues\n") fout.write("Tim\t1;2;3\n") - with MetricReader.open(FakeMetric, fpath_to_read) as reader: + with ModelReader.open(FakeMetric, fpath_to_read) as reader: metrics = list(reader) assert len(metrics) == 1 @@ -77,8 +77,8 @@ class FakeMetric(Metric): # Test writing fpath_to_write = tmp_path / "written.txt" - writer: MetricWriter[FakeMetric] - with MetricWriter.open(FakeMetric, fpath_to_write) as writer: + writer: ModelWriter[FakeMetric] + with ModelWriter.open(FakeMetric, fpath_to_write) as writer: writer.write(metrics[0]) with fpath_to_write.open("r") as f: @@ -97,8 +97,8 @@ class FakeMetric(Metric): # Test writing fpath_to_write = tmp_path / "written.txt" - writer: MetricWriter[FakeMetric] - with MetricWriter.open(FakeMetric, fpath_to_write) as writer: + writer: ModelWriter[FakeMetric] + with ModelWriter.open(FakeMetric, fpath_to_write) as writer: writer.write(FakeMetric(name="Clint", values=[0.1, 0.002, 0.00301])) with fpath_to_write.open("r") as f: @@ -125,7 +125,7 @@ class FakeMetric(Metric): fout.write("Nils\t\n") fout.write("Tim\t1,2,3\n") - with MetricReader.open(FakeMetric, fpath_to_read) as reader: + with ModelReader.open(FakeMetric, fpath_to_read) as reader: metrics = list(reader) assert len(metrics) == 2 @@ -136,8 +136,8 @@ class FakeMetric(Metric): # Test writing fpath_to_write = tmp_path / "written.txt" - writer: MetricWriter[FakeMetric] - with MetricWriter.open(FakeMetric, fpath_to_write) as writer: + writer: ModelWriter[FakeMetric] + with ModelWriter.open(FakeMetric, fpath_to_write) as writer: writer.writeall(metrics) with fpath_to_write.open("r") as f: @@ -179,14 +179,14 @@ class FakeMetric(Metric): p = tmp_path / "metrics.csv" # `.csv` infers a comma delimiter metric = FakeMetric(name="alice", tags=["x", "y", "z"]) - with MetricWriter.open(FakeMetric, p) as writer: + with ModelWriter.open(FakeMetric, p) as writer: writer.write(metric) # The list serializes to "x,y,z", whose commas collide with the comma delimiter, so csv # quote-protects the field on disk rather than splitting it into extra columns. assert p.read_text() == 'name,tags\nalice,"x,y,z"\n' - with MetricReader.open(FakeMetric, p) as reader: + with ModelReader.open(FakeMetric, p) as reader: assert list(reader) == [metric] @@ -208,7 +208,7 @@ class FakeMetric(Metric): fout.write("name\tfoo\tbar\n") fout.write("Nils\t1\t2\n") - with MetricReader.open(FakeMetric, fpath_to_read) as reader: + with ModelReader.open(FakeMetric, fpath_to_read) as reader: metrics = list(reader) assert len(metrics) == 1 @@ -219,8 +219,8 @@ class FakeMetric(Metric): # Test writing fpath_to_write = tmp_path / "written.txt" - writer: MetricWriter[FakeMetric] - with MetricWriter.open(FakeMetric, fpath_to_write) as writer: + writer: ModelWriter[FakeMetric] + with ModelWriter.open(FakeMetric, fpath_to_write) as writer: writer.write(FakeMetric(name="Tim", counts=Counter({FakeEnum.FOO: 3, FakeEnum.BAR: 4}))) with fpath_to_write.open("r") as f: @@ -245,13 +245,13 @@ class FakeMetric(Metric): # BAR is absent on construction; it must be written as 0, not as an empty cell. Otherwise the # header's "bar" column is left empty on disk and read-back fails parsing "" as an int. fpath = tmp_path / "test.txt" - writer: MetricWriter[FakeMetric] - with MetricWriter.open(FakeMetric, fpath) as writer: + writer: ModelWriter[FakeMetric] + with ModelWriter.open(FakeMetric, fpath) as writer: writer.write(FakeMetric(name="Nils", counts=Counter({FakeEnum.FOO: 5}))) assert fpath.read_text() == "name\tfoo\tbar\nNils\t5\t0\n" - with MetricReader.open(FakeMetric, fpath) as reader: + with ModelReader.open(FakeMetric, fpath) as reader: metrics = list(reader) assert metrics == [ @@ -296,7 +296,7 @@ class FakeMetric(Metric): fout.write("name\tfoo\n") fout.write("test\t5\n") - with MetricReader.open(FakeMetric, fpath) as reader: + with ModelReader.open(FakeMetric, fpath) as reader: metrics = list(reader) assert len(metrics) == 1 diff --git a/tests/test_compression.py b/tests/test_compression.py index fa3bb7a..a48c5b8 100644 --- a/tests/test_compression.py +++ b/tests/test_compression.py @@ -6,8 +6,8 @@ import pytest from fgmetric.metric import Metric -from fgmetric.metric_reader import MetricReader -from fgmetric.metric_writer import MetricWriter +from fgmetric.model_reader import ModelReader +from fgmetric.model_writer import ModelWriter class ExampleMetric(Metric): @@ -18,11 +18,11 @@ class ExampleMetric(Metric): def test_reader_open_reads_gzipped_file(tmp_path: Path) -> None: - """Test MetricReader.open transparently decompresses .gz files.""" + """Test ModelReader.open transparently decompresses .gz files.""" p = tmp_path / "metrics.tsv.gz" with gzip.open(p, mode="wt", encoding="utf-8") as f: f.write("name\tvalue\nalice\t1\nbob\t2\n") - with MetricReader.open(ExampleMetric, p) as reader: + with ModelReader.open(ExampleMetric, p) as reader: metrics = list(reader) assert metrics == [ ExampleMetric(name="alice", value=1), @@ -31,9 +31,9 @@ def test_reader_open_reads_gzipped_file(tmp_path: Path) -> None: def test_writer_open_writes_gzipped_file(tmp_path: Path) -> None: - """Test MetricWriter.open transparently compresses .gz files.""" + """Test ModelWriter.open transparently compresses .gz files.""" p = tmp_path / "out.tsv.gz" - with MetricWriter.open(ExampleMetric, p) as writer: + with ModelWriter.open(ExampleMetric, p) as writer: writer.write(ExampleMetric(name="alice", value=1)) with gzip.open(p, mode="rt", encoding="utf-8") as f: assert f.read() == "name\tvalue\nalice\t1\n" @@ -44,9 +44,9 @@ def test_round_trip_compressed(tmp_path: Path, ext: str) -> None: """Test write-then-read round trip across plain text and supported compression formats.""" p = tmp_path / f"metrics.tsv{ext}" expected = [ExampleMetric(name="alice", value=1), ExampleMetric(name="bob", value=2)] - with MetricWriter.open(ExampleMetric, p) as writer: + with ModelWriter.open(ExampleMetric, p) as writer: writer.writeall(expected) - with MetricReader.open(ExampleMetric, p) as reader: + with ModelReader.open(ExampleMetric, p) as reader: assert list(reader) == expected @@ -55,33 +55,33 @@ def test_round_trip_csv_with_inferred_delimiter(tmp_path: Path, ext: str) -> Non """Test that delimiter inference composes with compression suffixes on round trip.""" p = tmp_path / f"metrics.csv{ext}" expected = [ExampleMetric(name="alice", value=1), ExampleMetric(name="bob", value=2)] - with MetricWriter.open(ExampleMetric, p) as writer: + with ModelWriter.open(ExampleMetric, p) as writer: writer.writeall(expected) - with MetricReader.open(ExampleMetric, p) as reader: + with ModelReader.open(ExampleMetric, p) as reader: assert list(reader) == expected def test_gzipped_csv_is_comma_delimited(tmp_path: Path) -> None: """A .csv.gz file written without an explicit delimiter is comma-delimited inside.""" p = tmp_path / "out.csv.gz" - with MetricWriter.open(ExampleMetric, p) as writer: + with ModelWriter.open(ExampleMetric, p) as writer: writer.write(ExampleMetric(name="alice", value=1)) with gzip.open(p, mode="rt", encoding="utf-8") as f: assert f.read() == "name,value\nalice,1\n" def test_gzipped_file_is_actually_gzipped(tmp_path: Path) -> None: - """A .gz file written by MetricWriter.open has the gzip magic bytes.""" + """A .gz file written by ModelWriter.open has the gzip magic bytes.""" p = tmp_path / "out.tsv.gz" - with MetricWriter.open(ExampleMetric, p) as writer: + with ModelWriter.open(ExampleMetric, p) as writer: writer.write(ExampleMetric(name="alice", value=1)) assert p.read_bytes()[:2] == b"\x1f\x8b" def test_bz2_file_is_actually_bz2(tmp_path: Path) -> None: - """A .bz2 file written by MetricWriter.open has the bz2 magic bytes.""" + """A .bz2 file written by ModelWriter.open has the bz2 magic bytes.""" p = tmp_path / "out.tsv.bz2" - with MetricWriter.open(ExampleMetric, p) as writer: + with ModelWriter.open(ExampleMetric, p) as writer: writer.write(ExampleMetric(name="alice", value=1)) assert p.read_bytes()[:3] == b"BZh" # Sanity: stdlib decompresses cleanly. @@ -89,9 +89,9 @@ def test_bz2_file_is_actually_bz2(tmp_path: Path) -> None: def test_xz_file_is_actually_xz(tmp_path: Path) -> None: - """An .xz file written by MetricWriter.open has the xz magic bytes.""" + """An .xz file written by ModelWriter.open has the xz magic bytes.""" p = tmp_path / "out.tsv.xz" - with MetricWriter.open(ExampleMetric, p) as writer: + with ModelWriter.open(ExampleMetric, p) as writer: writer.write(ExampleMetric(name="alice", value=1)) assert p.read_bytes()[:6] == b"\xfd7zXZ\x00" # Sanity: stdlib decompresses cleanly. diff --git a/tests/test_metric.py b/tests/test_metric.py index f5d2e5d..5aa0f43 100644 --- a/tests/test_metric.py +++ b/tests/test_metric.py @@ -1,8 +1,8 @@ """ Tests for the `Metric.read` convenience classmethod. -`Metric.read` is a thin wrapper over `MetricReader.open`; parsing behavior is covered in -`test_metric_reader.py`. These tests cover only the wrapper contract: delegation of each +`Metric.read` is a thin wrapper over `ModelReader.open`; parsing behavior is covered in +`test_model_reader.py`. These tests cover only the wrapper contract: delegation of each keyword argument, accepted path types, and eager evaluation. """ diff --git a/tests/test_metric_reader.py b/tests/test_model_reader.py similarity index 82% rename from tests/test_metric_reader.py rename to tests/test_model_reader.py index daa26fe..0f61b95 100644 --- a/tests/test_metric_reader.py +++ b/tests/test_model_reader.py @@ -8,11 +8,11 @@ from pydantic import ValidationError from fgmetric import Metric -from fgmetric.metric_reader import MetricReader +from fgmetric.model_reader import ModelReader class ExampleMetric(Metric): - """Example Metric subclass used in MetricReader tests.""" + """Example Metric subclass used in ModelReader tests.""" name: str value: int @@ -20,8 +20,8 @@ class ExampleMetric(Metric): def test_reader_iterates_from_iterable() -> None: source = StringIO("name\tvalue\nalice\t1\nbob\t2\n") - reader = MetricReader(ExampleMetric, source) - assert_type(reader, MetricReader[ExampleMetric]) + reader = ModelReader(ExampleMetric, source) + assert_type(reader, ModelReader[ExampleMetric]) metrics = list(reader) assert_type(metrics, list[ExampleMetric]) assert metrics == [ @@ -32,7 +32,7 @@ def test_reader_iterates_from_iterable() -> None: def test_reader_supports_headerless_with_fieldnames() -> None: source = StringIO("alice\t1\nbob\t2\n") - reader = MetricReader(ExampleMetric, source, fieldnames=["name", "value"]) + reader = ModelReader(ExampleMetric, source, fieldnames=["name", "value"]) metrics = list(reader) assert metrics == [ ExampleMetric(name="alice", value=1), @@ -43,12 +43,12 @@ def test_reader_supports_headerless_with_fieldnames() -> None: def test_reader_rejects_fieldnames_matching_header_at_construction() -> None: source = StringIO("name\tvalue\nalice\t1\n") with pytest.raises(ValueError, match="appears to be a header"): - MetricReader(ExampleMetric, source, fieldnames=["name", "value"]) + ModelReader(ExampleMetric, source, fieldnames=["name", "value"]) def test_reader_does_not_close_caller_handle() -> None: source = StringIO("name\tvalue\nalice\t1\n") - reader = MetricReader(ExampleMetric, source) + reader = ModelReader(ExampleMetric, source) list(reader) assert not source.closed @@ -56,8 +56,8 @@ def test_reader_does_not_close_caller_handle() -> None: def test_open_reads_file(tmp_path: Path) -> None: p = tmp_path / "metrics.tsv" p.write_text("name\tvalue\nalice\t1\nbob\t2\n") - with MetricReader.open(ExampleMetric, p) as reader: - assert_type(reader, MetricReader[ExampleMetric]) + with ModelReader.open(ExampleMetric, p) as reader: + assert_type(reader, ModelReader[ExampleMetric]) metrics = list(reader) assert_type(metrics, list[ExampleMetric]) assert metrics == [ @@ -68,7 +68,7 @@ def test_open_reads_file(tmp_path: Path) -> None: def test_open_does_not_open_file_until_enter(tmp_path: Path) -> None: p = tmp_path / "missing.tsv" - cm = MetricReader.open(ExampleMetric, p) + cm = ModelReader.open(ExampleMetric, p) with pytest.raises(FileNotFoundError): with cm: pass @@ -77,16 +77,16 @@ def test_open_does_not_open_file_until_enter(tmp_path: Path) -> None: def test_open_requires_context_manager_usage(tmp_path: Path) -> None: p = tmp_path / "metrics.tsv" p.write_text("name\tvalue\nalice\t1\n") - cm = MetricReader.open(ExampleMetric, p) + cm = ModelReader.open(ExampleMetric, p) with pytest.raises(TypeError): list(cm) # type: ignore[call-overload] def test_open_infers_delimiter_from_extension(tmp_path: Path) -> None: - """Test that MetricReader.open reads a .csv file as comma-delimited by default.""" + """Test that ModelReader.open reads a .csv file as comma-delimited by default.""" p = tmp_path / "metrics.csv" p.write_text("name,value\nalice,1\nbob,2\n") - with MetricReader.open(ExampleMetric, p) as reader: + with ModelReader.open(ExampleMetric, p) as reader: metrics = list(reader) assert metrics == [ ExampleMetric(name="alice", value=1), @@ -98,7 +98,7 @@ def test_open_explicit_delimiter_overrides_inference(tmp_path: Path) -> None: """Test that an explicit delimiter wins over the extension-inferred one.""" p = tmp_path / "metrics.csv" p.write_text("name\tvalue\nalice\t1\n") - with MetricReader.open(ExampleMetric, p, delimiter="\t") as reader: + with ModelReader.open(ExampleMetric, p, delimiter="\t") as reader: metrics = list(reader) assert metrics == [ExampleMetric(name="alice", value=1)] @@ -108,49 +108,49 @@ def test_open_raises_for_uninferrable_delimiter(tmp_path: Path) -> None: p = tmp_path / "metrics.dat" p.write_text("name\tvalue\nalice\t1\n") with pytest.raises(ValueError, match="Could not infer a delimiter"): - with MetricReader.open(ExampleMetric, p): + with ModelReader.open(ExampleMetric, p): pass def test_open_respects_encoding(tmp_path: Path) -> None: - """Test that MetricReader.open decodes the file with the specified encoding.""" + """Test that ModelReader.open decodes the file with the specified encoding.""" p = tmp_path / "metrics.tsv" p.write_bytes("name\tvalue\nrené\t1\n".encode("latin-1")) - with MetricReader.open(ExampleMetric, p, encoding="latin-1") as reader: + with ModelReader.open(ExampleMetric, p, encoding="latin-1") as reader: metrics = list(reader) assert metrics == [ExampleMetric(name="rené", value=1)] def test_open_raises_file_not_found_for_missing_file(tmp_path: Path) -> None: - """MetricReader.open raises FileNotFoundError when the file does not exist.""" + """ModelReader.open raises FileNotFoundError when the file does not exist.""" with pytest.raises(FileNotFoundError, match="does not exist"): - with MetricReader.open(ExampleMetric, tmp_path / "missing.tsv"): + with ModelReader.open(ExampleMetric, tmp_path / "missing.tsv"): pass def test_open_raises_file_not_found_for_missing_compressed_file(tmp_path: Path) -> None: - """MetricReader.open raises FileNotFoundError for a missing compressed file too.""" + """ModelReader.open raises FileNotFoundError for a missing compressed file too.""" with pytest.raises(FileNotFoundError, match="does not exist"): - with MetricReader.open(ExampleMetric, tmp_path / "missing.tsv.gz"): + with ModelReader.open(ExampleMetric, tmp_path / "missing.tsv.gz"): pass def test_open_raises_is_a_directory_for_directory(tmp_path: Path) -> None: - """MetricReader.open raises IsADirectoryError when the path is a directory.""" + """ModelReader.open raises IsADirectoryError when the path is a directory.""" with pytest.raises(IsADirectoryError, match="is a directory"): - with MetricReader.open(ExampleMetric, tmp_path): + with ModelReader.open(ExampleMetric, tmp_path): pass def test_open_raises_permission_error_for_unreadable_file( tmp_path: Path, chmod: Callable[[Path, int], None] ) -> None: - """MetricReader.open raises PermissionError when the file is not readable.""" + """ModelReader.open raises PermissionError when the file is not readable.""" p = tmp_path / "metrics.tsv" p.write_text("name\tvalue\nalice\t1\n") chmod(p, 0o000) with pytest.raises(PermissionError, match="not readable"): - with MetricReader.open(ExampleMetric, p): + with ModelReader.open(ExampleMetric, p): pass @@ -225,7 +225,7 @@ def test_open_tsv(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tcount\nfoo\t1\n") - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: metrics = list(reader) assert len(metrics) == 1 @@ -238,7 +238,7 @@ def test_open_csv(tmp_path: Path) -> None: fpath = tmp_path / "metrics.csv" fpath.write_text("name,count\nfoo,1\n") - with MetricReader.open(SimpleMetric, fpath, delimiter=",") as reader: + with ModelReader.open(SimpleMetric, fpath, delimiter=",") as reader: metrics = list(reader) assert len(metrics) == 1 @@ -247,11 +247,11 @@ def test_open_csv(tmp_path: Path) -> None: def test_open_yields_correct_type(tmp_path: Path) -> None: - """Test that MetricReader yields instances of the correct Metric subclass.""" + """Test that ModelReader yields instances of the correct Metric subclass.""" fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tcount\nfoo\t1\n") - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: for metric in reader: assert_type(metric, SimpleMetric) assert isinstance(metric, SimpleMetric) @@ -262,7 +262,7 @@ def test_open_multiple_rows(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tcount\nfoo\t1\nbar\t2\nbaz\t3\n") - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: metrics = list(reader) assert len(metrics) == 3 @@ -284,7 +284,7 @@ def test_open_coerces_string_to_int(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tcount\nfoo\t42\n") - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: metrics = list(reader) assert metrics[0].count == 42 @@ -296,7 +296,7 @@ def test_open_coerces_string_to_float(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tscore\nfoo\t3.14\n") - with MetricReader.open(MetricWithFloat, fpath) as reader: + with ModelReader.open(MetricWithFloat, fpath) as reader: metrics = list(reader) assert metrics[0].score == pytest.approx(3.14) @@ -308,7 +308,7 @@ def test_open_coerces_string_to_bool(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tis_active\nfoo\ttrue\nbar\tfalse\n") - with MetricReader.open(MetricWithBool, fpath) as reader: + with ModelReader.open(MetricWithBool, fpath) as reader: metrics = list(reader) assert metrics[0].is_active is True @@ -325,7 +325,7 @@ def test_open_empty_field_becomes_none(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tvalue\nfoo\t\n") - with MetricReader.open(MetricWithOptional, fpath) as reader: + with ModelReader.open(MetricWithOptional, fpath) as reader: metrics = list(reader) assert metrics[0].value is None @@ -336,7 +336,7 @@ def test_open_empty_field_with_optional_type(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tvalue\nfoo\t\nbar\t42\n") - with MetricReader.open(MetricWithOptional, fpath) as reader: + with ModelReader.open(MetricWithOptional, fpath) as reader: metrics = list(reader) assert metrics[0].value is None @@ -349,7 +349,7 @@ def test_open_empty_field_with_required_type_raises(tmp_path: Path) -> None: fpath.write_text("name\tcount\nfoo\t\n") with pytest.raises(ValidationError): - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: list(reader) @@ -364,7 +364,7 @@ def test_open_handles_utf8_bom(tmp_path: Path) -> None: # Write file with BOM prefix fpath.write_bytes(b"\xef\xbb\xbfname\tcount\nfoo\t1\n") - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: metrics = list(reader) assert len(metrics) == 1 @@ -383,7 +383,7 @@ def test_open_with_field_alias(tmp_path: Path) -> None: # Header uses alias "count" instead of field name "read_count" fpath.write_text("name\tcount\nfoo\t100\n") - with MetricReader.open(MetricWithAlias, fpath) as reader: + with ModelReader.open(MetricWithAlias, fpath) as reader: metrics = list(reader) assert metrics[0].name == "foo" @@ -400,7 +400,7 @@ def test_open_empty_file_no_rows(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tcount\n") - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: metrics = list(reader) assert len(metrics) == 0 @@ -411,7 +411,7 @@ def test_open_file_not_found_raises(tmp_path: Path) -> None: fpath = tmp_path / "does_not_exist.tsv" with pytest.raises(FileNotFoundError): - with MetricReader.open(SimpleMetric, fpath): + with ModelReader.open(SimpleMetric, fpath): pass @@ -427,7 +427,7 @@ def test_open_missing_required_field_raises(tmp_path: Path) -> None: fpath.write_text("name\nfoo\n") with pytest.raises(ValidationError): - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: list(reader) @@ -437,7 +437,7 @@ def test_open_invalid_type_raises(tmp_path: Path) -> None: fpath.write_text("name\tcount\nfoo\tnot_an_int\n") with pytest.raises(ValidationError): - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: list(reader) @@ -446,7 +446,7 @@ def test_open_extra_columns_ignored(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("name\tcount\textra\nfoo\t1\tignored\n") - with MetricReader.open(SimpleMetric, fpath) as reader: + with ModelReader.open(SimpleMetric, fpath) as reader: metrics = list(reader) assert len(metrics) == 1 @@ -466,7 +466,7 @@ def test_open_headerless_with_fieldnames(tmp_path: Path) -> None: # Three data rows, no header fpath.write_text("foo\t1\nbar\t2\nbaz\t3\n") - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: metrics = list(reader) assert [m.name for m in metrics] == ["foo", "bar", "baz"] @@ -478,7 +478,7 @@ def test_open_headerless_with_alias(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("foo\t100\n") - with MetricReader.open(MetricWithAlias, fpath, fieldnames=["name", "count"]) as reader: + with ModelReader.open(MetricWithAlias, fpath, fieldnames=["name", "count"]) as reader: metrics = list(reader) assert metrics[0].name == "foo" @@ -491,7 +491,7 @@ def test_open_headerless_missing_required_field_raises(tmp_path: Path) -> None: fpath.write_text("foo\n") with pytest.raises(ValidationError): - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name"]) as reader: + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name"]) as reader: list(reader) @@ -502,7 +502,7 @@ def test_open_headerless_row_missing_column_raises(tmp_path: Path) -> None: fpath.write_text("foo\n") with pytest.raises(ValidationError): - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: list(reader) @@ -513,7 +513,7 @@ def test_open_headerless_short_row_among_valid_rows_raises(tmp_path: Path) -> No fpath.write_text("foo\t1\nbar\nbaz\t3\n") with pytest.raises(ValidationError): - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: list(reader) @@ -523,7 +523,7 @@ def test_open_headerless_row_extra_column_ignored(tmp_path: Path) -> None: # Row has an extra value not covered by fieldnames fpath.write_text("foo\t1\textra\n") - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: metrics = list(reader) assert len(metrics) == 1 @@ -538,7 +538,7 @@ def test_open_headerless_detects_header_row_raises(tmp_path: Path) -> None: fpath.write_text("name\tcount\nfoo\t1\n") with pytest.raises(ValueError, match="header"): - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]): + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]): pass @@ -549,7 +549,7 @@ def test_open_headerless_first_row_coincidentally_matching_value_is_data( fpath = tmp_path / "metrics.tsv" fpath.write_text("name\t1\nbar\t2\n") - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: metrics = list(reader) assert [m.name for m in metrics] == ["name", "bar"] @@ -564,7 +564,7 @@ def test_open_headerless_detects_header_row_with_aliased_field_raises(tmp_path: fpath.write_text("name\tcount\nfoo\t1\n") with pytest.raises(ValueError, match="header"): - with MetricReader.open(MetricWithAlias, fpath, fieldnames=["name", "count"]): + with ModelReader.open(MetricWithAlias, fpath, fieldnames=["name", "count"]): pass @@ -573,7 +573,7 @@ def test_open_headerless_empty_file(tmp_path: Path) -> None: fpath = tmp_path / "metrics.tsv" fpath.write_text("") - with MetricReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: + with ModelReader.open(SimpleMetric, fpath, fieldnames=["name", "count"]) as reader: metrics = list(reader) assert metrics == [] @@ -595,7 +595,7 @@ def test_open_parent_class_discards_subclass_fields(tmp_path: Path) -> None: # File has columns for ChildMetric but we read with ParentMetric fpath.write_text("name\tvalue\textra_field\tanother_field\nfoo\t42\textra\t99\n") - with MetricReader.open(ParentMetric, fpath) as reader: + with ModelReader.open(ParentMetric, fpath) as reader: metrics = list(reader) assert len(metrics) == 1 diff --git a/tests/test_metric_writer.py b/tests/test_model_writer.py similarity index 73% rename from tests/test_metric_writer.py rename to tests/test_model_writer.py index 2cf4610..85aa019 100644 --- a/tests/test_metric_writer.py +++ b/tests/test_model_writer.py @@ -12,8 +12,8 @@ from pytest_mock import MockerFixture from fgmetric import Metric -from fgmetric import MetricWriter -from fgmetric import metric_writer +from fgmetric import ModelWriter +from fgmetric import model_writer class FakeMetric(Metric): @@ -27,9 +27,9 @@ def test_writer(tmp_path: Path) -> None: """Test we can write a Metric to file.""" fpath = tmp_path / "test.txt" - writer: MetricWriter[FakeMetric] - with MetricWriter.open(FakeMetric, fpath) as writer: - assert_type(writer, MetricWriter[FakeMetric]) + writer: ModelWriter[FakeMetric] + with ModelWriter.open(FakeMetric, fpath) as writer: + assert_type(writer, ModelWriter[FakeMetric]) writer.write(FakeMetric(foo="abc", bar=1)) writer.write(FakeMetric(foo="def", bar=2)) @@ -42,7 +42,7 @@ def test_writer(tmp_path: Path) -> None: def test_writer_with_counter_metric(tmp_path: Path) -> None: - """Test we can write a Counter metric through MetricWriter.""" + """Test we can write a Counter metric through ModelWriter.""" @unique class FakeEnum(StrEnum): @@ -55,7 +55,7 @@ class CounterMetric(Metric): fpath = tmp_path / "test.txt" - with MetricWriter.open(CounterMetric, fpath) as writer: + with ModelWriter.open(CounterMetric, fpath) as writer: writer.write(CounterMetric(name="test", counts=Counter({FakeEnum.FOO: 3, FakeEnum.BAR: 4}))) with fpath.open("r") as f: @@ -68,7 +68,7 @@ class CounterMetric(Metric): def test_writer_accepts_text_io_and_writes_to_it() -> None: """A writer constructed with a TextIO sink writes to that sink.""" sink = StringIO() - writer = MetricWriter(FakeMetric, sink) + writer = ModelWriter(FakeMetric, sink) writer.write(FakeMetric(foo="abc", bar=1)) assert sink.getvalue() == "foo\tbar\nabc\t1\n" @@ -76,14 +76,14 @@ def test_writer_accepts_text_io_and_writes_to_it() -> None: def test_writer_writes_header_at_construction() -> None: """The header row is written immediately on construction.""" sink = StringIO() - MetricWriter(FakeMetric, sink) + ModelWriter(FakeMetric, sink) assert sink.getvalue() == "foo\tbar\n" def test_writer_does_not_close_caller_handle() -> None: """A caller-supplied sink is not closed by the writer.""" sink = StringIO() - writer = MetricWriter(FakeMetric, sink) + writer = ModelWriter(FakeMetric, sink) writer.write(FakeMetric(foo="abc", bar=1)) assert not sink.closed @@ -96,7 +96,7 @@ class AliasMetric(Metric): read_count: int = Field(alias="count") sink = StringIO() - writer = MetricWriter(AliasMetric, sink) + writer = ModelWriter(AliasMetric, sink) # `read_count` must be populated via its alias. writer.write(AliasMetric(name="foo", count=100)) assert sink.getvalue() == "name\tcount\nfoo\t100\n" @@ -116,7 +116,7 @@ class ListMetric(Metric): tags: list[int] sink = StringIO() - writer = MetricWriter(ListMetric, sink, delimiter=",") + writer = ModelWriter(ListMetric, sink, delimiter=",") writer.write(ListMetric(name="x", tags=[1, 2, 3])) # Columns are separated by the writer's "," delimiter; elements inside the `tags` cell are # joined by the ";" collection_delimiter. @@ -126,32 +126,32 @@ class ListMetric(Metric): def test_writer_uses_custom_lineterminator() -> None: """A custom line terminator ends both the header and the rows.""" sink = StringIO() - writer = MetricWriter(FakeMetric, sink, lineterminator="\r\n") + writer = ModelWriter(FakeMetric, sink, lineterminator="\r\n") writer.write(FakeMetric(foo="abc", bar=1)) assert sink.getvalue() == "foo\tbar\r\nabc\t1\r\n" def test_writer_open_writes_header_and_rows(tmp_path: Path) -> None: - """Test that MetricWriter.open opens a file, writes the header, and writes rows.""" + """Test that ModelWriter.open opens a file, writes the header, and writes rows.""" p = tmp_path / "out.tsv" - with MetricWriter.open(FakeMetric, p) as writer: + with ModelWriter.open(FakeMetric, p) as writer: writer.write(FakeMetric(foo="abc", bar=1)) assert p.read_text() == "foo\tbar\nabc\t1\n" def test_writer_open_does_not_touch_file_until_enter(tmp_path: Path) -> None: - """Test that MetricWriter.open does not open the file until the context is entered.""" + """Test that ModelWriter.open does not open the file until the context is entered.""" p = tmp_path / "out.tsv" p.write_text("existing content\n") - MetricWriter.open(FakeMetric, p) + ModelWriter.open(FakeMetric, p) # Construction alone must not truncate or rewrite the file. assert p.read_text() == "existing content\n" def test_writer_open_infers_delimiter_from_extension(tmp_path: Path) -> None: - """Test that MetricWriter.open writes a .csv file as comma-delimited by default.""" + """Test that ModelWriter.open writes a .csv file as comma-delimited by default.""" p = tmp_path / "out.csv" - with MetricWriter.open(FakeMetric, p) as writer: + with ModelWriter.open(FakeMetric, p) as writer: writer.write(FakeMetric(foo="abc", bar=1)) assert p.read_text() == "foo,bar\nabc,1\n" @@ -159,7 +159,7 @@ def test_writer_open_infers_delimiter_from_extension(tmp_path: Path) -> None: def test_writer_open_explicit_delimiter_overrides_inference(tmp_path: Path) -> None: """Test that an explicit delimiter wins over the extension-inferred one.""" p = tmp_path / "out.csv" - with MetricWriter.open(FakeMetric, p, delimiter="\t") as writer: + with ModelWriter.open(FakeMetric, p, delimiter="\t") as writer: writer.write(FakeMetric(foo="abc", bar=1)) assert p.read_text() == "foo\tbar\nabc\t1\n" @@ -168,64 +168,64 @@ def test_writer_open_raises_for_uninferrable_delimiter(tmp_path: Path) -> None: """Test that an unrecognized extension raises when no delimiter is given.""" p = tmp_path / "out.dat" with pytest.raises(ValueError, match="Could not infer a delimiter"): - with MetricWriter.open(FakeMetric, p): + with ModelWriter.open(FakeMetric, p): pass # The file must not be created when inference fails. assert not p.exists() def test_writer_open_respects_encoding(tmp_path: Path) -> None: - """Test that MetricWriter.open writes with the specified encoding.""" + """Test that ModelWriter.open writes with the specified encoding.""" p = tmp_path / "out.tsv" - with MetricWriter.open(FakeMetric, p, encoding="latin-1") as writer: + with ModelWriter.open(FakeMetric, p, encoding="latin-1") as writer: writer.write(FakeMetric(foo="rené", bar=1)) assert p.read_bytes() == "foo\tbar\nrené\t1\n".encode("latin-1") def test_writer_open_closes_owned_file(tmp_path: Path, mocker: MockerFixture) -> None: - """MetricWriter.open closes the file it owns when the context exits.""" - spy = mocker.spy(metric_writer, "xopen") + """ModelWriter.open closes the file it owns when the context exits.""" + spy = mocker.spy(model_writer, "xopen") p = tmp_path / "out.tsv" - with MetricWriter.open(FakeMetric, p) as writer: + with ModelWriter.open(FakeMetric, p) as writer: writer.write(FakeMetric(foo="abc", bar=1)) # `open` opened exactly one file; spy_return is that handle, which must now be closed. assert spy.spy_return.closed def test_writer_open_closes_owned_file_on_exception(tmp_path: Path, mocker: MockerFixture) -> None: - """MetricWriter.open closes the file it owns even when the context body raises.""" - spy = mocker.spy(metric_writer, "xopen") + """ModelWriter.open closes the file it owns even when the context body raises.""" + spy = mocker.spy(model_writer, "xopen") p = tmp_path / "out.tsv" with pytest.raises(RuntimeError, match="boom"): - with MetricWriter.open(FakeMetric, p) as writer: + with ModelWriter.open(FakeMetric, p) as writer: writer.write(FakeMetric(foo="abc", bar=1)) raise RuntimeError("boom") assert spy.spy_return.closed def test_writer_open_raises_file_not_found_for_missing_parent(tmp_path: Path) -> None: - """MetricWriter.open raises FileNotFoundError when the parent directory does not exist.""" + """ModelWriter.open raises FileNotFoundError when the parent directory does not exist.""" with pytest.raises(FileNotFoundError, match="does not exist"): - with MetricWriter.open(FakeMetric, tmp_path / "missing" / "out.tsv"): + with ModelWriter.open(FakeMetric, tmp_path / "missing" / "out.tsv"): pass def test_writer_open_raises_is_a_directory_for_directory(tmp_path: Path) -> None: - """MetricWriter.open raises IsADirectoryError when the path is a directory.""" + """ModelWriter.open raises IsADirectoryError when the path is a directory.""" with pytest.raises(IsADirectoryError, match="is a directory"): - with MetricWriter.open(FakeMetric, tmp_path): + with ModelWriter.open(FakeMetric, tmp_path): pass def test_writer_open_raises_permission_error_for_readonly_file( tmp_path: Path, chmod: Callable[[Path, int], None] ) -> None: - """MetricWriter.open raises PermissionError when the target file is not writable.""" + """ModelWriter.open raises PermissionError when the target file is not writable.""" p = tmp_path / "out.tsv" p.write_text("locked\n") chmod(p, 0o444) with pytest.raises(PermissionError, match="not writable"): - with MetricWriter.open(FakeMetric, p): + with ModelWriter.open(FakeMetric, p): pass # The check must fire before the file is opened, so the contents are untouched. assert p.read_text() == "locked\n" @@ -234,30 +234,30 @@ def test_writer_open_raises_permission_error_for_readonly_file( def test_writer_open_raises_permission_error_for_readonly_parent( tmp_path: Path, chmod: Callable[[Path, int], None] ) -> None: - """MetricWriter.open raises PermissionError when the parent directory is not writable.""" + """ModelWriter.open raises PermissionError when the parent directory is not writable.""" d = tmp_path / "readonly" d.mkdir() chmod(d, 0o555) with pytest.raises(PermissionError, match="not writable"): - with MetricWriter.open(FakeMetric, d / "out.tsv"): + with ModelWriter.open(FakeMetric, d / "out.tsv"): pass def test_writer_open_refuses_to_overwrite_existing_file_by_default(tmp_path: Path) -> None: - """MetricWriter.open refuses to clobber an existing file unless `overwrite=True`.""" + """ModelWriter.open refuses to clobber an existing file unless `overwrite=True`.""" p = tmp_path / "out.tsv" p.write_text("existing\n") with pytest.raises(FileExistsError, match="already exists"): - with MetricWriter.open(FakeMetric, p): + with ModelWriter.open(FakeMetric, p): pass # The refusal must fire before the file is opened, so the contents are untouched. assert p.read_text() == "existing\n" def test_writer_open_overwrites_existing_file_when_overwrite_true(tmp_path: Path) -> None: - """With `overwrite=True`, MetricWriter.open truncates and rewrites an existing file.""" + """With `overwrite=True`, ModelWriter.open truncates and rewrites an existing file.""" p = tmp_path / "out.tsv" p.write_text("stale\tcontent\nto be replaced\n") - with MetricWriter.open(FakeMetric, p, overwrite=True) as writer: + with ModelWriter.open(FakeMetric, p, overwrite=True) as writer: writer.write(FakeMetric(foo="abc", bar=1)) assert p.read_text() == "foo\tbar\nabc\t1\n" diff --git a/tests/test_record_model.py b/tests/test_record_model.py new file mode 100644 index 0000000..04fd147 --- /dev/null +++ b/tests/test_record_model.py @@ -0,0 +1,84 @@ +""" +Tests for `RecordModel`, the mixin-free base class. + +`RecordModel` performs the default tabular parsing only: one model field per column, with no +converter behaviors (null sentinels, delimited lists, counter pivot tables). These tests cover +that base contract and contrast it with `Metric`, which layers the converters on top. +""" + +from pathlib import Path +from typing import assert_type + +from pydantic import Field + +from fgmetric import Metric +from fgmetric import ModelReader +from fgmetric import ModelWriter +from fgmetric import RecordModel + + +class PlainRecord(RecordModel): + """A plain record with one field per column.""" + + name: str + note: str | None + + +class AliasRecord(RecordModel): + """A record whose field serializes under an alias.""" + + name: str + read_count: int = Field(alias="count") + + +class SentinelMetric(Metric): + """A `Metric` mirroring `PlainRecord`, used to contrast converter behavior.""" + + name: str + note: str | None + + +def test_header_fieldnames_are_plain_columns() -> None: + """Test `_header_fieldnames` returns one column per field, in declaration order.""" + assert PlainRecord._header_fieldnames() == ["name", "note"] + + +def test_header_fieldnames_respect_alias() -> None: + """Test `_header_fieldnames` resolves each field to its serialized (aliased) name.""" + assert AliasRecord._header_fieldnames() == ["name", "count"] + + +def test_does_not_apply_null_sentinels() -> None: + """Test `RecordModel` leaves empty Optional fields untouched (no null-sentinel converter).""" + record = PlainRecord.model_validate({"name": "a", "note": ""}) + assert record.note == "" + + +def test_metric_applies_null_sentinels_where_record_model_does_not() -> None: + """Test the same empty Optional field becomes `None` on `Metric` but `""` on `RecordModel`.""" + record = PlainRecord.model_validate({"name": "a", "note": ""}) + metric = SentinelMetric.model_validate({"name": "a", "note": ""}) + assert record.note == "" + assert metric.note is None + + +def test_read_classmethod(tmp_path: Path) -> None: + """Test the inherited `read` classmethod parses a file into the calling subclass.""" + p = tmp_path / "records.tsv" + p.write_text("name\tnote\na\tfoo\n") + records = PlainRecord.read(p) + assert_type(records, list[PlainRecord]) + assert records == [PlainRecord(name="a", note="foo")] + + +def test_roundtrip_through_model_reader_and_writer(tmp_path: Path) -> None: + """Test a bare `RecordModel` round-trips through `ModelWriter` and `ModelReader`.""" + p = tmp_path / "records.tsv" + records = [PlainRecord(name="a", note="foo"), PlainRecord(name="b", note=None)] + + with ModelWriter.open(PlainRecord, p) as writer: + writer.writeall(records) + + with ModelReader.open(PlainRecord, p) as reader: + # An empty cell deserializes to "" on a plain RecordModel, not None. + assert list(reader) == [PlainRecord(name="a", note="foo"), PlainRecord(name="b", note="")] diff --git a/tests/test_roundtrip.py b/tests/test_roundtrip.py index a4bfdf0..27a60cf 100644 --- a/tests/test_roundtrip.py +++ b/tests/test_roundtrip.py @@ -4,8 +4,8 @@ from pathlib import Path from fgmetric import Metric -from fgmetric import MetricReader -from fgmetric import MetricWriter +from fgmetric import ModelReader +from fgmetric import ModelWriter class SimpleMetric(Metric): @@ -30,10 +30,10 @@ def test_roundtrip_scalar_optional(tmp_path: Path) -> None: ] p = tmp_path / "metrics.tsv" - with MetricWriter.open(OptionalScalarMetric, p) as writer: + with ModelWriter.open(OptionalScalarMetric, p) as writer: writer.writeall(expected) - with MetricReader.open(OptionalScalarMetric, p) as reader: + with ModelReader.open(OptionalScalarMetric, p) as reader: assert list(reader) == expected @@ -56,21 +56,21 @@ class CounterMetric(Metric): ] p = tmp_path / "metrics.tsv" - with MetricWriter.open(CounterMetric, p) as writer: + with ModelWriter.open(CounterMetric, p) as writer: writer.writeall(expected) - with MetricReader.open(CounterMetric, p) as reader: + with ModelReader.open(CounterMetric, p) as reader: assert list(reader) == expected def test_roundtrip_empty_file(tmp_path: Path) -> None: """Writing zero rows produces a header-only file that reads back to an empty list.""" p = tmp_path / "metrics.tsv" - with MetricWriter.open(SimpleMetric, p) as writer: + with ModelWriter.open(SimpleMetric, p) as writer: writer.writeall([]) # The writer emits the header on context entry, so the file is header-only. assert p.read_bytes() == b"name\tvalue\n" - with MetricReader.open(SimpleMetric, p) as reader: + with ModelReader.open(SimpleMetric, p) as reader: assert list(reader) == [] From 5497ccdc73d65de89c4a9790ae8cbe6f388a3f4a Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Tue, 16 Jun 2026 13:23:32 -0400 Subject: [PATCH 2/3] refactor: simplify Counter header guard and document RecordModel Follow-up to code review: consolidate CounterPivotTable._header_fieldnames onto a single _counter_fieldname guard (matching the file's assert-narrowing idiom) and document the MRO/inheritance contract for future header-overriding converters. Add a README section introducing RecordModel and when to use it over Metric. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 13 ++++++++++++ fgmetric/converters/_counter_pivot_table.py | 22 +++++++++++---------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index b5ff2ad..a370a26 100644 --- a/README.md +++ b/README.md @@ -229,6 +229,19 @@ class BaseCountMetric(Metric): # BaseCountMetric(position=1, counts=Counter({Base.A: 10, Base.C: 5, ...})) ``` +### Plain records (`RecordModel`) + +`Metric` is a batteries-included subclass of `RecordModel`, the mixin-free base. `RecordModel` does the default tabular parsing only — one model field per column — without the converter behaviors above (empty-string-to-`None`, delimited lists, counter pivots). Subclass it directly when you want plain field-per-column parsing and nothing else; `ModelReader`, `ModelWriter`, and the inherited `read()` work on any `RecordModel`. + +```python +from fgmetric import RecordModel + + +class SampleRecord(RecordModel): + name: str + note: str | None # an empty cell stays "" here; on a Metric it would become None +``` + ## Contributing See the [contributing guide](./CONTRIBUTING.md) for development setup and testing instructions. diff --git a/fgmetric/converters/_counter_pivot_table.py b/fgmetric/converters/_counter_pivot_table.py index 1147dfd..e3549f5 100644 --- a/fgmetric/converters/_counter_pivot_table.py +++ b/fgmetric/converters/_counter_pivot_table.py @@ -18,7 +18,9 @@ # NB: This mixin inherits `RecordModel` rather than `BaseModel` (unlike the other converters) so # that it can override `RecordModel._header_fieldnames` and call `super()` to reuse the default # alias-aware header. It is the only converter that rewrites the on-disk column set, so it is the -# only one that needs the base header contract in scope. +# only one that needs the base header contract in scope. Any future converter that also overrides +# `_header_fieldnames` must likewise inherit `RecordModel` (so `super()` type-checks) and appear +# before it in the model's MRO (so `super()` resolves to the base header, not `BaseModel`). class CounterPivotTable(RecordModel): """ A mixin to support pivot table representations of Counters. @@ -228,18 +230,18 @@ def _header_fieldnames(cls) -> list[str]: # -> ["name", "red", "green", "blue"] ``` """ - # The Counter field has no alias (aliases are rejected for Counter fields), so its - # serialized name equals its field name; drop it from the default header. - fieldnames = [ - name for name in super()._header_fieldnames() if name != cls._counter_fieldname - ] + default_header = super()._header_fieldnames() - if cls._counter_enum is None: + if cls._counter_fieldname is None: # Short circuit if we don't have a Counter field - return fieldnames + return default_header - # Replace the dropped Counter field with one column per enum member - return fieldnames + [member.value for member in cls._counter_enum] + # The Counter field has no alias (aliases are rejected for Counter fields), so its + # serialized name equals its field name; drop it from the default header and replace it + # with one column per enum member. + assert cls._counter_enum is not None # not None iff _counter_fieldname is not None + non_counter = [name for name in default_header if name != cls._counter_fieldname] + return non_counter + [member.value for member in cls._counter_enum] @final @model_validator(mode="before") From cf49b31ac28d6efcb40046aa22a0a8637e18ce74 Mon Sep 17 00:00:00 2001 From: Matt Stone Date: Mon, 13 Jul 2026 16:19:26 -0400 Subject: [PATCH 3/3] refactor: address review feedback on RecordModel Split `RecordModel._header_fieldnames` into a thin overridable method plus a `@final` `_default_header_fieldnames` helper, so header-rewriting mixins (`CounterPivotTable`) build on the helper rather than `super()` and no longer depend on MRO ordering; the old MRO contract was enforced only by a comment. Also document that `T | None` fields do not round-trip `None` on the mixin-free base, extract the shared read-path `DEFAULT_ENCODING`, correct the misleading "mirrors ModelReader.open" comment on `read` (its "\t" delimiter default does not infer from the file extension), and mark the breaking CHANGELOG entries. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 8 ++--- fgmetric/converters/_counter_pivot_table.py | 11 +++---- fgmetric/model_reader.py | 5 ++- fgmetric/record_model.py | 36 +++++++++++++++++---- tests/test_record_model.py | 11 +++++++ 5 files changed, 54 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40fe249..aea4646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,11 @@ All notable changes to this project will be documented in this file. ### Changed -- Rename `MetricReader`/`MetricWriter` to `ModelReader`/`ModelWriter`; the IO classes now read and write any `RecordModel` subclass, not just `Metric` (#10) -- `ModelWriter` is IO-first; open a path with `ModelWriter.open()` rather than passing it to the constructor (#42) -- `Metric.read` is now a thin wrapper over `ModelReader.open` and reads eagerly, returning a `list` instead of a lazy generator; it accepts `str` paths in addition to `Path` and gains transparent decompression and the `encoding` kwarg. Use `ModelReader.open` to stream metrics without holding them all in memory (#62) +- **Breaking:** Rename `MetricReader`/`MetricWriter` to `ModelReader`/`ModelWriter`; the IO classes now read and write any `RecordModel` subclass, not just `Metric` (#10) +- **Breaking:** `ModelWriter` is IO-first; open a path with `ModelWriter.open()` rather than passing it to the constructor (#42) +- **Breaking:** `Metric.read` is now a thin wrapper over `ModelReader.open` and reads eagerly, returning a `list` instead of a lazy generator; it accepts `str` paths in addition to `Path` and gains transparent decompression and the `encoding` kwarg. Use `ModelReader.open` to stream metrics without holding them all in memory (#62) - `ModelReader.open` and `ModelWriter.open` eagerly validate that the path is readable/writable on context entry (#66) -- `ModelWriter.open` now refuses to overwrite an existing file by default, raising `FileExistsError`; pass `overwrite=True` to truncate and overwrite it (#69) +- **Breaking:** `ModelWriter.open` now refuses to overwrite an existing file by default, raising `FileExistsError`; pass `overwrite=True` to truncate and overwrite it (#69) ## [0.3.0] - 2026-05-12 diff --git a/fgmetric/converters/_counter_pivot_table.py b/fgmetric/converters/_counter_pivot_table.py index e3549f5..436d526 100644 --- a/fgmetric/converters/_counter_pivot_table.py +++ b/fgmetric/converters/_counter_pivot_table.py @@ -16,11 +16,10 @@ # NB: This mixin inherits `RecordModel` rather than `BaseModel` (unlike the other converters) so -# that it can override `RecordModel._header_fieldnames` and call `super()` to reuse the default -# alias-aware header. It is the only converter that rewrites the on-disk column set, so it is the -# only one that needs the base header contract in scope. Any future converter that also overrides -# `_header_fieldnames` must likewise inherit `RecordModel` (so `super()` type-checks) and appear -# before it in the model's MRO (so `super()` resolves to the base header, not `BaseModel`). +# that `_default_header_fieldnames` is in scope for its `_header_fieldnames` override. It is the +# only converter that rewrites the on-disk column set. Any future converter that also overrides +# `_header_fieldnames` should build on `_default_header_fieldnames` (not `super()`) so the result +# does not depend on MRO ordering. class CounterPivotTable(RecordModel): """ A mixin to support pivot table representations of Counters. @@ -230,7 +229,7 @@ def _header_fieldnames(cls) -> list[str]: # -> ["name", "red", "green", "blue"] ``` """ - default_header = super()._header_fieldnames() + default_header = cls._default_header_fieldnames() if cls._counter_fieldname is None: # Short circuit if we don't have a Counter field diff --git a/fgmetric/model_reader.py b/fgmetric/model_reader.py index b806955..9c2a76c 100644 --- a/fgmetric/model_reader.py +++ b/fgmetric/model_reader.py @@ -16,6 +16,9 @@ if TYPE_CHECKING: from fgmetric.record_model import RecordModel +DEFAULT_ENCODING = "utf-8-sig" +"""Default read encoding; strips a UTF-8 BOM so Excel-exported CSVs open cleanly.""" + class ModelReader[T: RecordModel]: """ @@ -78,7 +81,7 @@ def open( path: Path | str, delimiter: str | None = None, fieldnames: Sequence[str] | None = None, - encoding: str = "utf-8-sig", + encoding: str = DEFAULT_ENCODING, ) -> Iterator[Self]: """ Open `path` and yield a `ModelReader` over its contents. diff --git a/fgmetric/record_model.py b/fgmetric/record_model.py index 278fa28..b3cb628 100644 --- a/fgmetric/record_model.py +++ b/fgmetric/record_model.py @@ -2,9 +2,11 @@ from collections.abc import Sequence from pathlib import Path from typing import Self +from typing import final from pydantic import BaseModel +from fgmetric.model_reader import DEFAULT_ENCODING from fgmetric.model_reader import ModelReader @@ -25,6 +27,13 @@ class RecordModel(BaseModel, ABC): Subclasses should define their fields using Pydantic field annotations. + Note: + `RecordModel` applies no null-sentinel handling, so `T | None` fields do not round-trip + `None`. On write, `None` serializes to an empty cell; on read, that empty cell comes back + as `""` - for a `str | None` field it validates to `""` (not `None`), and for a non-string + optional (e.g. `int | None`) it raises a `ValidationError`. Use `Metric`, or add the + `NullSentinels` mixin, if you need `None` preserved across a write/read cycle. + Example: ```python class AlignmentRecord(RecordModel): @@ -42,10 +51,11 @@ class AlignmentRecord(RecordModel): def read( cls, path: Path | str, - # NB: these defaults mirror `ModelReader.open()`; keep them in sync. + # NB: `encoding` mirrors `ModelReader.open`. Unlike `open`, `read` does not infer the + # delimiter from the file extension - it defaults to a literal tab. delimiter: str = "\t", fieldnames: Sequence[str] | None = None, - encoding: str = "utf-8-sig", + encoding: str = DEFAULT_ENCODING, ) -> list[Self]: """ Read all instances from a file path. @@ -109,8 +119,9 @@ def _header_fieldnames(cls) -> list[str]: the model's field names when aliases are used. Subclasses and mixins (e.g., `CounterPivotTable`) may override this method to adjust the - header to match a custom serializer. Such overrides should call `super()` to obtain the - default field-per-column header and then transform it. + header to match a custom serializer. Such overrides should build on + `_default_header_fieldnames` (the plain field-per-column header) rather than `super()`, + so they do not depend on their position in the model's MRO. Note: This method is deliberately not used during reading/validation; see @@ -121,9 +132,22 @@ def _header_fieldnames(cls) -> list[str]: """ # TODO: support returning the set of fields that would be constructed if the class has a # custom model serializer + return cls._default_header_fieldnames() + + @final + @classmethod + def _default_header_fieldnames(cls) -> list[str]: + """ + Return the plain field-per-column header: one column per field, in declaration order. - # Resolve each field to the key it serializes under (its alias, when one is set), so the - # header matches the keys produced by `model_dump(by_alias=True)`. + Each field resolves to the key it serializes under (its alias, when one is set), so the + header matches the keys produced by `model_dump(by_alias=True)`. This method is `@final` + so header-rewriting mixins can call it directly instead of routing through `super()`, + which would otherwise depend on MRO ordering. + + Returns: + The list of fieldnames to use as the header row. + """ return [ info.serialization_alias or info.alias or name for name, info in cls.model_fields.items() diff --git a/tests/test_record_model.py b/tests/test_record_model.py index 04fd147..6f5952b 100644 --- a/tests/test_record_model.py +++ b/tests/test_record_model.py @@ -48,6 +48,17 @@ def test_header_fieldnames_respect_alias() -> None: assert AliasRecord._header_fieldnames() == ["name", "count"] +def test_default_header_fieldnames_matches_header_fieldnames() -> None: + """ + Test `_default_header_fieldnames` is the plain header `_header_fieldnames` delegates to. + + Header-rewriting mixins build on this `@final` helper instead of `super()`, so it must return + the same alias-aware, field-per-column header as the default `_header_fieldnames`. + """ + assert PlainRecord._default_header_fieldnames() == PlainRecord._header_fieldnames() + assert AliasRecord._default_header_fieldnames() == ["name", "count"] + + def test_does_not_apply_null_sentinels() -> None: """Test `RecordModel` leaves empty Optional fields untouched (no null-sentinel converter).""" record = PlainRecord.model_validate({"name": "a", "note": ""})