Skip to content

refactor(corekit): normalise sentinel object names to SCREAMING_SNAKE #937

Closed

Description

@JarryShaw

The four sentinel objects in pcapkit/corekit/sentinels.py carry three different casings, while all four types already follow CamelCase + Type. Normalise the objects to SCREAMING_SNAKE.

The owner's ruling, verbatim (from #719):

take SCREAMING_SNAKE and accept the breaking change (no backport needed). we can change _ABSENT to ABSENT just document it as private type/class in the documentation and not for public use is enough.

So:

current becomes
NoValue NO_VALUE
_Absent ABSENT — leading underscore dropped
_AbsentType AbsentType
NULL, NO_DEFAULT, NullType, NoValueType, NoDefaultType unchanged

No deprecation alias. The break is accepted deliberately — NoValue is in pcapkit.corekit.fields.field.__all__ as of #911, so import * callers are affected.

Blast radius, measured (grep -rnw over pcapkit/ and tests/):

NoValue      109 references across 21 files
NoValueType   55 references across 13 files
_Absent       41 references across  5 files
_AbsentType   26 references across  5 files

plus docs/source/pcapkit/corekit/fields/{field,index,misc}.rst, docs/source/contributing/conventions.rst, and the sentinels.rst page that landed in #936.

Three consequences worth handling explicitly, not incidentally:

  1. Dropping the underscore makes ABSENT/AbsentType look public to tooling. They must stay out of __all__ — test_sentinel_exports_unit.py::test_the_private_sentinel_is_exported_neither_way pins that, and should keep pinning it under the new names. Privacy becomes documentation-only, per the ruling, so Sphinx will now pick them up where _-prefixed names were hidden; decide whether sentinels.rst documents them as explicitly-private or keeps omitting them. docs(corekit): add the sentinels API page so conventions.rst references resolve (#934) #936 omitted _Absent because it was underscore-private, so that reasoning needs restating either way.
  2. conventions.rst needs an object-naming rule added. :152 today states only "Keep the sentinel object's type class naming as <SENTINEL>Type" — it governs the type, never the object, which is the gap that let them diverge. The new rule belongs there.
  3. _expected_type_name in test_sentinel_exports_unit.py should collapse. Its bare.isupper() branch and its leading-underscore branch both exist only to cope with the inconsistency; after this, <SENTINEL>Type is one mechanical rule and the helper's special cases become dead.

Blocked on #932, which owns pcapkit/corekit/enum.py (a NO_DEFAULT/sentinel re-export shim), docs/source/contributing/conventions.rst, and tests/protocols/internet/test_mh_unit.py — all three in this change's path. Starts once #932 merges.

Activity

  1. added
    breakingBreaks public-facing behaviour or API (apply alongside the type label)
    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 30, 2026
  2. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 30, 2026
  3. JarryShaw commented on Sep 30, 2026

    @JarryShaw
    OwnerAuthor

    Closing: the rename is on main, verified rather than assumed. #939 merged as part of 83c7552b8; main now carries ABSENT = AbsentType() at pcapkit/corekit/sentinels.py:489 and NO_VALUE throughout.

    All three renames landed as ruled — NoValue → NO_VALUE, _Absent → ABSENT, _AbsentType → AbsentType, with no deprecation alias, per "take SCREAMING_SNAKE and accept the breaking change (no backport needed)".

    The three consequences this issue flagged were each handled rather than left to chance:

    • ABSENT/AbsentType are documented as expressly private on sentinels.rst, with the ruling quoted — not omitted. Omission would have been underscore-hiding by another name, which is what dropping the underscore moved away from. Both stay out of every __all__; sentinels.__all__ is ['NULL','NO_VALUE','NO_DEFAULT'], field.__all__ is ['NO_VALUE','Field'], protocol.__all__ is ['ProtocolBase'].
    • conventions.rst gained the object-naming rule it previously lacked — it had governed only the type class, which is the gap that let three casings diverge.
    • _expected_type_name collapsed to one mechanical rule, with strength preserved by a new regex test pinning ^[A-Z][A-Z0-9_]*$ across all four objects independently of type derivation.

    Two findings from the work worth keeping, both reusable: isort reordered five files because NO_VALUE sorts before Field/FieldBase under order_by_type — a pure case change moving imports is not obvious. And pylint's cyclic-import output is nondeterministic: 236 differing lines across two runs of the unmodified baseline, so R0401 churn can never be attributed to a diff.

    One thing that was my error, not this work's: I declared tests/protocols/internet/test_mh_unit.py off-limits to the worker after measuring "zero sentinel references in mh.py" — the source, never the test. The test had three, and #939's first head broke all five Python legs with ImportError: cannot import name 'NoValue'. The worker stopped and reported rather than crossing the boundary I drew, which was right.

  4. added this to the 1.5 milestone on Oct 6, 2026
  5. moved this to Done in PyPCAPKiton Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    breakingBreaks public-facing behaviour or API (apply alongside the type label)refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions