Skip to content

How should output be structured? #168

@liamhuber

Description

@liamhuber

A bunch of the new content since mid-December takes the idea of atomistics.shared.Output and really runs with it, using the following paradigm:

  • Define a new dataclass (template) to specify what fields are required
  • Define functions to actually do science
  • Use the functions to make a new "engine"
  • Define another new dataclass (actually gets used) and populate its callables with properties of the new engine

This creates an awful lot of code duplication, which is one of the main things the Output class was introduced to fix. It also feels very over-engineered: in many (all?) of the new cases, there is no need for an engine since there is only a single pure-python "engine" defined, and that's all we'll ever need.

In #166 I show how the two dataclass and engine can all be rolled into a single class, so that (when an engine is not truly required), the pattern looks like this:

  • Define functions to actually do science
  • Define a new output class

We could, in principle, jam the science functions into the class itself as well, but I am in favour of the current structure where each one is broken out as an individual function for the sake of downstream developers who may want to import them individually.

Then I thought about how we can abstract this. What do we want Output to really promise? IMO we want:

  • A predefined set of names for collections of meaningful data
    • Accessible at the class level!
  • A helper method to easily grab some subset of this collection

With this in mind, in #167 I made a new, truly abstract Output class to codify and document these requirements, then introduce a new abstract class for cases where we don't need an intermediate engine, and re-named the current Output to EngineOutput and left it functioning as-is.

Code duplication adds to the weight of maintenance, makes it harder for new devs to learn the code base, and increases the chance of a bug going unnoticed; we should aim for abstractions whenever they can be made without introducing significant complexity.

IMO the new PropertyEngine and the pattern of #166 can be applied to the rest of the new, verbose features that got added. But before jamming that through I thought we should take the elastic case as a chance to reflect on what (if any) changes need to be made to the solution in #167, or whether some totally different attack is called for.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions