Add format module

[DavidMStraub]

No description provided.

[Copilot]

Pull request overview

Adds a new gedcom7.format module that formats typed payload values back into GEDCOM 7 payload strings (the inverse of gedcom7.cast), exposes a new public entrypoint gedcom7.format_value, and introduces a comprehensive test suite covering formatter behavior and cast/format round-trips.

Changes:

• Introduce gedcom7/format.py with per-datatype formatter functions and a format_value() dispatcher keyed off const.payloads.
• Export format_value from gedcom7/__init__.py as part of the public API.
• Add test/test_format.py to validate formatting, round-trip invariants against casting, and corpus-based regression checks.

Reviewed changes

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

File	Description
test/test_format.py	Adds formatter unit tests, round-trip tests, and a corpus sweep over maximal70.ged.
gedcom7/format.py	New formatting implementation mirroring cast.CAST_FUNCTIONS and validating output against grammar regexes.
gedcom7/init.py	Exposes format_value in the package public surface.

Suppressed comments (1)

test/test_format.py:52

• Longitude tests only cover whole-degree values as ints (180/-180), but cast_value returns floats for longitude payloads too. Add float cases so the tests cover formatting of casted values.

        (-1e-05, "W0.00001"),
        (180, "E180"),
        (-180, "W180"),
    ],

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

gedcom7/format.py:51

• format_value() falls back to treating unknown payload types as plain strings, which means pointer payload structure types (those whose payload in const.payloads looks like @<...>@) are not validated. That makes it easy to accidentally serialize an invalid pointer value as text (and have it escaped as a line string later). Consider detecting pointer payload types and validating against grammar.pointer before returning.

    format_function = FORMAT_FUNCTIONS.get(payload)
    if not format_function:
        return _format_string(value)
    return format_function(value)

gedcom7/format.py:35

• The format_value() docstring says it returns None when a structure carries no payload, but some legal payloads can format to the empty string (e.g., an empty DatePeriod). It would help callers if the docstring explicitly mentions that "" can be returned for “empty but valid” payloads and should be treated differently from None.

    Returns ``None`` when the structure carries no payload, which for a boolean
    also means the structure itself should be left out: a false ``Y|<NULL>`` is
    written by omitting the structure, not by writing an empty payload.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

test/test_format.py:563

• This corpus sweep claims to “format every payload”, but the implementation skips structures with an empty payload string (it checks structure.text truthiness). If the intent is to skip empty payloads, the docstring should say so; otherwise the guard should be adjusted to include empty payloads once casting preserves them.

    """Format every payload in maximal70.ged and compare against the original."""

test/test_format.py:534

• The tests/docs assert a strict cast<->format inverse for “every data type”, but the current parsing side collapses empty payload strings to None (see gedcom7/cast.py: cast_value returns None when text is empty). That means empty-but-meaningful payloads like an empty DatePeriod (which this test file treats as a legal value) cannot round-trip through cast_value, and the inverse claim becomes false. Consider adding an explicit round-trip case for an empty DatePeriod (e.g. type NO-DATE with payload "") and adjusting cast.cast_value (and/or GedcomStructure.value) so empty payloads that are valid for the payload grammar are preserved rather than always mapped to None.

This issue also appears on line 563 of the same file.

    """Casting a formatted value gives the value back.

    This is the direction that is exact for every data type. Casting a payload
    is what discards detail, so once a value has been through it the pair is a
    true inverse, PersonalName included.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.