Skip to content

refactor(corekit): move pcapkit.utilities.chardet into corekit #1712

Closed as not planned

Description

@JarryShaw

Describe the bug
pcapkit.utilities.chardet (detect, the bounded-cache wrapper around chardet) is used by corekit/fields/strings.py:8, protocols/protocol.py:48, protocols/application/ftp.py:29 and httpv1.py:44, and is re-exported from pcapkit/utilities/__init__.py:15. The owner wants it in pcapkit.corekit.

Expected behavior

  • The module moves under pcapkit.corekit and its importers follow.
  • pcapkit.utilities.chardet and the pcapkit.utilities.detect re-export stay importable, so the move is not a breaking change.
  • docs/source/pcapkit/utilities/chardet.rst moves to the corekit docs.

Blocked until #1707 and #1706 merge, since both edit corekit/fields/strings.py.

Activity

  1. added
    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
    on Oct 14, 2026
  2. moved this to Blocked in PyPCAPKiton Oct 14, 2026
  3. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    The ruling on #1715 applies here too: the move updates every import path directly, with no re-export left at pcapkit/utilities/chardet.py.

  4. added
    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 Oct 14, 2026
  5. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    #1707 has merged. A worker is on it, with no re-export at the old path (the ruling on #1715). In strings.py it changes only the import line, so it does not collide with #1706.

  6. moved this from Blocked to WIP in PyPCAPKiton Oct 14, 2026
  7. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    I'm kinda debating what's the criteria for corekit or utilities. Not sure if chardet and many other modules under corekit and utilities should live.

  8. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    Understood. I am holding #1729, the chardet move, until the criterion is settled. Next I will post an inventory of every module in pcapkit/corekit and pcapkit/utilities: what each imports from pcapkit, who imports it, and whether it carries packet or protocol knowledge. Each module gets a proposed rule applied to it, so the decision is about concrete moves rather than names.

  9. added
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 14, 2026
  10. moved this from WIP to Needs decision in PyPCAPKiton Oct 14, 2026
  11. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    Inventory of every module in both packages, on main (fb7027b25). The import order today is already utilities (all of it), then corekit, then protocols/const, with one exception.

    Proposed rule:

    • utilities holds cross-cutting services: reporting (exceptions, warnings, logging), configuration, compatibility shims and wrappers around third-party libraries. It imports nothing from pcapkit outside utilities, at top level, inside a function or under TYPE_CHECKING. It carries no wire format or protocol behaviour.
    • corekit holds the types the packet model is built from: what protocols, const and foundation subclass, instantiate or decorate with. It may import utilities, and reaches protocols only lazily.

    Applied, only one module moves:

    The alternative, a "no packet knowledge" test, would also move multidict, version, module, sentinels, io, infoclass, enum and packet into utilities. That is about 8 breaking moves, each dragging the others, and leaves corekit with only fields, protochain, context, packet and byteorder.

    Your call: which rule? Under the proposed one, should #1729 close (chardet stays put), and where should decorators go, corekit or protocols?

  12. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    explain better about the options and criteria for the split of each option

  13. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    Three options. Each comes down to one question asked of a module.

    Option Question that decides utilities gets corekit gets Breaking moves
    A. Services vs types (recommended) Is it a service any code could use (reporting, config, compat, third-party wrapper) that imports nothing from pcapkit outside utilities? Then utilities; otherwise corekit. exceptions, warnings, logging, compat, chardet, config fields, infoclass, enum, multidict, module, sentinels, version, io, packet, context, protochain, decorators 1: decorators
    B. Packet knowledge Does it know about packets, wire formats or protocols? Yes means corekit, no means utilities. A's list plus multidict, version, module, sentinels, io, infoclass, enum fields, protochain, context, packet, byteorder, decorators 8: the seven listed plus decorators
    C. Who uses it Is it used only by the packet model (fields, protocols)? Then corekit. exceptions, warnings, logging, compat, config A's list plus chardet 2: decorators, and chardet (#1729)

    decorators moves under all three. It works on Protocol._file and imports protocol classes, so it fails every utilities test.

    The trade-offs:

    • A: the rule is checkable by import graph alone, since utilities never imports corekit. It also churns least.
    • B: cleanest by meaning. But the seven modules must move together, because infoclass imports enum and multidict, and enum and module import sentinels. The public types Info, MultiDict, EnumRegistry and VersionInfo would change path.
    • C: keeps your earlier call that chardet belongs with the fields. Its question is the fuzziest, though: compat and exceptions are also used mostly by the packet model, yet stay put.

    All three are cycle-safe as moved.

  14. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    okay, i think we should take A

  15. JarryShaw commented on Oct 14, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: option A.

    • utilities holds cross-cutting services: exceptions, warnings, logging, compat, config and third-party wrappers. It imports nothing from pcapkit outside utilities.
    • corekit holds the types the packet model is built from.

    What this means for the open work:

  16. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 14, 2026
  17. moved this from Needs decision to Done in PyPCAPKiton Oct 14, 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

    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions