Skip to content

Move type definitions out of the central lib/types folder and next to their code #892

Description

@bencap

Summary

Most of the backend's shared type definitions sit in one central folder, even though each is used by a single module or a single package. That means a reader has to open a second place to see the shape a function returns, and the central folder imports from across the codebase, which makes import cycles more likely. This issue moves each type next to the code that uses it, keeps a shared home only for the one type that is used everywhere, and writes the placement rule into the contributor instructions.

Placement rule

Put each type at the narrowest scope that covers all of its users:

  1. Used by one module: define it in that module.
  2. Used by several modules in one package: put it in a module that all of them already import, or in a new module named for what the types describe (schema.py for external API payloads, as in lib/mapping/schema.py). Don't add a types.py to a package out of habit.
  3. Used by unrelated packages: put it in a small shared module named for its domain.

Scope

Moves:

Symbols Used by New home
ResourceWithCreationModificationDates lib/annotation/contribution.py lib/annotation/contribution.py
PublicationIdentifierAssociations lib/annotation/method.py lib/annotation/method.py
SequenceFeature lib/annotation/proposition.py lib/annotation/proposition.py
ClassificationDict lib/score_calibrations.py lib/score_calibrations.py
MappingEntry, MappingEntries lib/uniprot/id_mapping.py lib/uniprot/id_mapping.py
EntityType lib/permissions/core.py, lib/permissions/utils.py lib/permissions/models.py
All of lib/types/clingen.py lib/clingen/services.py, lib/clingen/content_constructors.py new lib/clingen/schema.py
JobDefinition, PipelineDefinition lib/workflow, worker/jobs/registry.py, scripts/run_job.py, scripts/run_score_set_pipelines.py lib/workflow/definitions.py
JobExecutionOutcome about 20 modules in worker/, lib/logging/canonical.py, scripts new lib/workflow/outcome.py

Stays shared:

  • UserData is used by about 27 modules across routers/ and lib/. Move it from lib/types/authentication.py to a new lib/principal.py, keeping its model imports under TYPE_CHECKING, then delete lib/types/.

Unchanged:

  • worker/lib/managers/types.py is already shared only within its own package.
  • Types already defined in the module that uses them, such as those in routers/score_sets.py, lib/mapping/schema.py and lib/validation/transform.py.

Steps:

  1. Land this after the release branches merge into main. All of them touch these imports.
  2. Move the types and update every import directly, with no re-exports left in the old locations.
  3. Commit the JobExecutionOutcome rewrite separately so its diff reviews on its own.
  4. Update the worker docs that point at lib/types/workflow.py: worker/README.md, worker/job_registry.md, worker/jobs_overview.md and .github/instructions/worker.instructions.md.
  5. Add the placement rule above to .github/instructions/python.instructions.md.
  6. Move tests/worker/lib/managers/test_types.py and other tests of the moved types so they match the new module paths.
  7. Run ruff format only on the files you touch.

Acceptance criteria

  • Each symbol in the moves table is defined in its new home and nowhere else.
  • UserData is defined in lib/principal.py.
  • src/mavedb/lib/types/ no longer exists.
  • git grep "lib.types\|lib/types" -- src tests .github returns no matches.
  • No types.py module is added to any package.
  • lib/clingen/schema.py imports nothing from other lib/clingen modules.
  • lib/workflow/definitions.py imports nothing from job_factory or pipeline_factory.
  • .github/instructions/python.instructions.md states the placement rule.
  • Importing mavedb.lib.logging.canonical, mavedb.lib.permissions, mavedb.lib.principal and mavedb.lib.workflow in a fresh interpreter raises no circular-import error.
  • The test suite and mypy pass.
Background
  • Why not centralize every type. A central types folder groups code by kind instead of by feature, so each feature spreads across two directories. It also has to import models from across the codebase: lib/types/annotation.py and lib/types/permissions.py already do this at runtime, and lib/types/authentication.py needed a TYPE_CHECKING guard to avoid a cycle. The API isn't installed as a library by other applications, so sharing types outside the codebase isn't a reason to centralize them.
  • Why UserData doesn't go in lib/authentication.py. That module imports deps and orcid. Every permissions module imports UserData, so putting it there would make permissions load the request-dependency stack.
  • Why JobExecutionOutcome gets its own module. It's a dataclass with factory methods and to_dict(), a domain object rather than a type hint.
  • Logging and ORM models. lib/workflow/__init__.py eagerly imports JobFactory and PipelineFactory, which load ORM models. Importing lib/workflow/outcome.py runs that __init__, so lib/logging/canonical.py will load models. This isn't a cycle. If logging should stay model-free, make the import inside its isinstance check lazy.
  • Verification. Every usage in the moves table was checked on main, release-2026.3.0, the calibration-controls branch (Add calibration controls to support clinical confidence in variant interpretation #754) and the allele-centric mapping branch. SequenceFeature is used by lib/annotation/util.py on the first three and by lib/annotation/proposition.py on the allele-centric branch. The table uses the merged state.

Activity

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

    app: backendTask implementation touches the backendapp: workerTask implementation touches the worker

    Type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions