-
Notifications
You must be signed in to change notification settings - Fork 2
Description
At present, we accept two flavors of score ranges: Brnich style ranges with odds paths and calibration style Zeiberg ranges. For the Brnich style ranges, each range really has the same underlying data to display the range and the difference lies in the source used to generate them. This differs from the implication of how ranges are stored in MaveDB: under an explicitly named key in the range dictionary.
This isn't really a problem for now, but as we continue to develop the database we'll likely want to accept various flavors of supported range styles. This will require a bit of a rework to the Pydantic container that validates the ranges, but should be possible. Right now, each range is defined explicitly and expected under a unique key in the container:
mavedb-api/src/mavedb/view_models/score_range.py
Lines 384 to 390 in d088e1a
| class SavedScoreSetRanges(ScoreSetRangesBase): | |
| record_type: str = None # type: ignore | |
| investigator_provided: Optional[SavedInvestigatorScoreRanges] = None | |
| pillar_project: Optional[SavedPillarProjectScoreRanges] = None | |
| _record_type_factory = record_type_validator()(set_record_type) |
We might consider retaining certain reserved keys while forcing arbitrary key/value pairs in the dictionary validate their values against all possible range definitions. We'd retain the ability to easily access certain important ranges (such as investigator provided ranges) while gaining the flexibility to accept other arbitrarily named ranges and/or calibrations users may want to supply for certain data sets.
This might be possible by using the Pydantic RootModel as below, but I haven't really explored it.
RangeTypes = Union[SavedInvestigatorScoreRanges, SavedZeibergCalibrationScoreRanges]
class RangeModel(RootModel[Dict[str, RangeTypes]]):
investigator_provided: Optional[SavedInvestigatorScoreRanges] = None
zeiberg_calibration: Optional[SavedZeibergCalibrationScoreRanges] = None
range_data = {
"investigator_provided": <Instance of SavedInvestigatorScoreRanges>,
"arbitraryRange1": <Instance of SavedInvestigatorScoreRanges>,
"arbitraryRange2": <Instance of SavedZeibergCalibrationScoreRanges>,
"zeiberg_calibration": None,
}
range_instance = RangeModel(range_data)
We should:
- Allow additional calibrations with custom names, validated against existing calibration models.
- Consider whether the utility of retaining statically named calibrations for certain calibration types important for site function is worth it. This mostly comes down to ease of type checking and always knowing that, for instance, the investigator provided ranges are of the correct type.
- If we don't anticipate this being a large issue, we might consider just transitioning this dictionary to a list.
- Add validation to ensure generic calibrations keys don't collide with existing names.
- Display all calibrations in the UI in the following order: (1) Statically named calibrations non-research calibrations, (2) Generic non-research calibrations, (3) research only calibrations.