DM-48282: Refactor obscore to allow subclasses #1142
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #1142 +/- ##
==========================================
- Coverage 89.50% 89.47% -0.04%
==========================================
Files 366 366
Lines 49015 49065 +50
Branches 5941 5950 +9
==========================================
+ Hits 43872 43899 +27
- Misses 3728 3748 +20
- Partials 1415 1418 +3 ☔ View full report in Codecov by Sentry. |
| @@ -96,7 +99,7 @@ class RecordFactory: | |||
| Registry dimensions universe. | |||
| spatial_plugins : `~collections.abc.Collection` of `SpatialObsCorePlugin` | |||
| Spatial plugins. | |||
| exposure_region_factory : `ExposureRegionFactory`, optional | |||
| derived_region_factory : `DerivedRegionFactory`, optional | |||
| Manager for Registry dimensions. | |||
There was a problem hiding this comment.
Looks like I copy-pasted this line long time ago and did not update, can you add a better description?
| @@ -120,15 +123,122 @@ def __init__( | |||
| self.visit = universe["visit"] | |||
| self.physical_filter = cast(Dimension, universe["physical_filter"]) | |||
|
|
|||
| @abstractmethod | |||
| def region_dimension(self, dimensions: DimensionGroup) -> tuple[str | None, str | None]: | |||
There was a problem hiding this comment.
Could just one of them be None? If not then tuple[str, str] | tuple[None, None] could be better?
| @classmethod | ||
| def get_record_type_from_universe(cls, universe: DimensionUniverse) -> type[RecordFactory]: |
There was a problem hiding this comment.
We could potentially specify the type in the obscore config itself?
| spatial_plugins : `~collections.abc.Collection` of `SpatialObsCorePlugin` | ||
| Spatial plugins. | ||
| derived_region_factory : `DerivedRegionFactory`, optional | ||
| Manager for Registry dimensions. |
There was a problem hiding this comment.
This too wants an update.
| # should be used for the RecordFactory for this universe. | ||
| record_factory_type = func() | ||
| if not issubclass(record_factory_type, RecordFactory): | ||
| raise ValueError( |
Checklist
doc/changesconfigs/old_dimensions