AAVSO / VStar

VStar is a visualisation and analysis tool for variable star data brought to you by AAVSO
https://www.aavso.org/vstar
GNU Affero General Public License v3.0
9 stars 3 forks source link

User created series do not persist across additive loads #339

Closed dbenn closed 1 year ago

dbenn commented 1 year ago

To reproduce this:

sinusoid1.csv sinusoid2.csv

dbenn commented 1 year ago

Attention will need to be paid to preserving these collections in ObservationPlotModel across additive loads:

    /**
     * A mapping from series number to a list of observations where each such
     * list is a data series.
     */
    protected Map<Integer, List<ValidObservation>> seriesNumToObSrcListMap;

    /**
     * A mapping from series number to source type.
     */
    protected Map<Integer, SeriesType> seriesNumToSrcTypeMap;

    /**
     * A mapping from source type to series number.
     */
    protected Map<SeriesType, Integer> srcTypeToSeriesNumMap;

We already pass a mapping from SeriesType to List<ValidObservation> in the ObservationAndMeanPlotModel constructor. For addObs == true in https://github.com/AAVSO/VStar/blob/master/src/org/aavso/tools/vstar/ui/mediator/Mediator.java#L1260 we may need to pay attention to these being updated/included via ObservationAndMeanPlotModel and so ObservationPlotModel.

I suspect that all the machinery that is needed already exists in ObservationPlotModel and that we are very close to what is required.

Requires some rumination however.

dbenn commented 1 year ago

I suspect the user defined series and their obs are not being propagated to the Mediator's validObservationCategoryMap, so ObservationPlotModel's. Will verify.

Actually, validObservationCategoryMap in Mediator is being updated:

    protected Listener<SeriesCreationMessage> createSeriesCreationListener() {
        return new Listener<SeriesCreationMessage>() {
            @Override
            public void update(SeriesCreationMessage info) {
                validObservationCategoryMap.put(info.getType(), info.getObs());
                validObsList = obsInserter.addValidObservations(info.getObs());
            }

            @Override
            public boolean canBeRemoved() {
                return false;
            }
        };
    }
dbenn commented 1 year ago

Okay. What's happening is that the validObservationCategoryMap is being overwritten when an additive load occurs, so information about previously loaded series is lost from the map, including user-defined series. This leads to the follow-on effect when the incomplete map is passed to the ObservationPlotModel constructor.

I have added code that appears to fix this. I'm testing it and will commit it soon.

I'm surprised I did not run into this problem before now!

EDIT: the flow on effects were broader than I thought as per commits below.

dbenn commented 1 year ago

Not obvious from the commit messages is that I also fixed a bug in which user defined series obs were not showing up in the obs list pane!