search

DOI-USGS / ale

Abstraction Layer for Ephemerides (ALE)
Other
13 stars 32 forks source link

Problems with Ale's architecture. #108

Closed Kelvinrr closed 5 years ago

Kelvinrr commented 5 years ago

The goal with the Ale driver structure is to, as close as possible, resemble a form where you fill out how to map label keys and ancillary data structures to Ale ISD keys (i.e. ISIS translation tables++). Ale seems to be at least 90% there, but there are some perceived structural problems with Ale's driver structure. Some identified so far:

  1. There exist an implicit division between classes that retrieve data directly from labels/spice (Pds3, NaifSpice, etc.) and classes that define how those data are packed in the ISD (Distortion Models, Camera Types, etc.). We should probably make this more explicit. Possible solutions: segregating them into different modules or fusing the two into the same method (i.e., make all methods return Ale ISD compatible keys. We currently using properties prefixed with _ to read from data structures, the rest of the properties return the ale compatible keys).

  2. Similar to 1, we should probably do something to formalize the difference between classes read from labels and classes that read from secondary data sources that rely on the label (Naif Spice/ISIS Spice tables).

  3. Ale has a convoluted inheritance structure in some places. Some drivers like to move common denominators into classes that others inherit from (e.g., spacecraft_id, common spice calls, etc.). Hierarchal mixins seem like a code smell. Possible solution: duplicate code as a lesser evil? Not sure with this one.

I don't believe these require any significant reworking, but perhaps, we should address them before moving too far forward.

I think I got the big ones. If anyone else has other structural concerns, probably best to bring them up here, or maybe there is nothing wrong, and we can close this and move forward.

jessemapel commented 5 years ago

I think the re-architecting addressed all of these