ARTIST-Association / ARTIST-Association/ARTIST

Refactor: Reduce import coupling and clarify HDF5 loading responsibilities (IO vs. domain vs. orchestration)

Open
#208 0 comments 0 reactions 1 assignee Claimed by @mcw92 View on GitHub
maintenance
Dominant language
Python
Stars
12
Forks
8
PR merge metrics
No merged PRs in 30d

Description

## Summary

The current codebase is tightly coupled due to a combination of:
- domain classes implementing `from_hdf5()` methods,
- centralized registries importing concrete implementations,
- and distributed HDF5 parsing logic across domain and IO modules.

This leads to circular imports and unclear ownership of object construction.

---

## Problem

We currently mix three concerns:

1. **Domain representation (what things are)**
- e.g. `HeliostatField`, `Sun`, `Surface`

2. **Persistence logic (how things are stored/loaded)**
- HDF5 parsing: `from_hdf5()` methods scattered across domain classes

3. **Orchestration (how things are assembled)**
- scenario and field assembly logic
- type dispatch via registries

This combination causes:
- circular imports (IO ↔ domain ↔ registry cycles)
- heavy package initialization dependencies
- unclear separation between "what an object is" and "how it is loaded"

---

## Current Pain Points

- `type_registry` imports concrete domain classes.
- Domain objects import IO modules for loading.
- Scenario/field loaders depend on many submodules.
- HDF5 loading is distributed across multiple layers.

---

## Proposed Direction

Move complex `from_hdf5()` logic for composite objects into explicit loader functions, like:

```python
load_heliostat_field_from_hdf5(...)
load_scenario_from_hdf5(...)
```

while keeping `from_hdf5()` for simple, self-contained domain objects.

## Goal

- Make dependency flow one-directional (IO → domain).
- Remove circular imports caused by bidirectional coupling.
- Centralize orchestration logic without creating a monolithic loader.
- Keep domain objects lightweight and focused on representation.

## Non-goals

- No full removal of `from_hdf5()` methods
- No single global IO “god module”
- No change to domain model structure beyond loading responsibilities

## Suggested steps
- Move orchestration-heavy `from_hdf5()` methods into `io/hdf5/loaders/`.
- Keep simple `from_hdf5()` methods on leaf domain objects.
- Refactor `type_registry` to avoid importing concrete implementations.
- Reduce package-level imports that trigger large dependency graphs.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.