Closed tpolecat closed 6 years ago
We should probably squash-merge this because nothing prior to the last commit will pass CI.
Let me know if any of my responses alarmed you. If not I'll merge.
Let me know if any of my responses alarmed you. If not I'll merge.
No alarms and no surprises. Merge away. 👍
This is big PR, sorry, but relatively straightforward.
This removes indexed types (and indeed all parameterization in the main model) in favor of simpler (but more verbose) ADTs with cases per instrument.
Program
is now a simple case class.Observation
is now an ADT with constructors for each instrument. SoObservation.GmosN
is a GMOS North observation and all its data members are specialized to GMOS North.TargetEnvironment
is now an ADT with constructors for each instrument.Asterism
is now an ADT with constructors for each asterism type (one per instrument now, but there will be more).StaticConfig
andDynamicConfig
are no longer indexed on the singleton type of the instrument.Step
is now an ADT with constructors for each instrument, and the sense is reversed with respect to the "base" step types likeBias
andScience
, which are now members of each constructor rather than the reverse. This lets us say things liketype GmosSequence = List[Step.GmosN]
for instance.Self
type member and matchingAux
pattern which were added to support singleton indexing ofgem.enum
types are no longer necessary and have been removed.The data layer is largely the same, with a few exceptions:
observation
and no longer need theAsterismType
hint. This simplifies things downstream but means the query may become complex as more asterism types are added. But I think it's the right way to do it.Some misc. cleanup is technically unrelated but aided in the refactoring.
SmartGcal
is now in its own source file undergem.config
Instrument
enumeration. Uses ofF2
,GmosNorth
, andGmosSouth
have been normalized toFlamingos2
,GmosN
, andGmosS
, respectively so everything matches up.TreeMap
andTreeSet
, by copy/pasting the existing instances forSortedMap
andSortedSet
, respectively.Open questions, to be addressed later:
List[Step]
in the model but remain asTreeMap[Location.Middle, Step]
in most of the data access layer. We may want to push the map representation all the way up.TreeSet
andTreeMap
should probably becomeSortedMap
andSortedSet
, respectively, if possible, to take advantage of existing instances provided by Cats.Followup tasks:
Gnirs
isn't on the list of observation types we generate for our tests, so I added it but it caused some test failures so I removed it again. So we need to track that down.Instrument
enum (and all pattern blocks derived from it) are unordered. Let's sort them by instrument name.