You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #18 added strict mypy type hints across the codebase, introducing ~20 assert statements to narrow X | None types. These asserts are a code smell — they indicate that the current two-phase initialisation pattern (construct object, then configure it later) leaves attributes as None longer than necessary.
Root cause
Several classes use a pattern where attributes are initialised as None and populated later:
Lattice.params is set to None in __init__, then assigned via Simulation.run()
Site.p_neighbours is set to None in __init__, then populated by Lattice.__init__
Simulation.lattice, Simulation.atoms, etc. are None until run() is called
This means methods that use these attributes need assert self.x is not None guards, even though in practice the attributes are always set by the time the methods are called.
Desired outcome
Restructure data flow so that objects receive their required data at construction time, eliminating the need for nullable attributes and assert guards. This likely involves:
Passing SimulationParameters to Lattice at construction (or to methods that need it)
Ensuring p_neighbours is always populated (possibly by making it a required constructor parameter)
Rethinking Simulation so that its properties don't depend on state set during run()
Summary
PR #18 added strict mypy type hints across the codebase, introducing ~20
assertstatements to narrowX | Nonetypes. These asserts are a code smell — they indicate that the current two-phase initialisation pattern (construct object, then configure it later) leaves attributes asNonelonger than necessary.Root cause
Several classes use a pattern where attributes are initialised as
Noneand populated later:Lattice.paramsis set toNonein__init__, then assigned viaSimulation.run()Site.p_neighboursis set toNonein__init__, then populated byLattice.__init__Simulation.lattice,Simulation.atoms, etc. areNoneuntilrun()is calledThis means methods that use these attributes need
assert self.x is not Noneguards, even though in practice the attributes are always set by the time the methods are called.Desired outcome
Restructure data flow so that objects receive their required data at construction time, eliminating the need for nullable attributes and assert guards. This likely involves:
SimulationParameterstoLatticeat construction (or to methods that need it)p_neighboursis always populated (possibly by making it a required constructor parameter)Simulationso that its properties don't depend on state set duringrun()Related PRs
Optionsinto a dataclass and refactoring mutable counters are natural places to address this