oxyplot / oxyplot

A cross-platform plotting library for .NET
https://oxyplot.github.io/
MIT License
3.28k stars 960 forks source link

Make `PlotModel` serializable #730

Open TobiScw opened 8 years ago

TobiScw commented 8 years ago

Hey, I created an Annotation GUI that allows to add annotations to a chart. And I'd like to save the state of a chart for later editing. For that it is neccessary to serialize Annotations and Series, which is currently not possible because most of the serialisation solutions require a parameterless constructor in each class. An other option would be to add serialisation attributes to Series and Annotations and their sub-Elements.

Do you think that one option would be feasible? Do you have alternative ideas? Saving a chart's state should be a useful feature.

tibel commented 8 years ago

I don't think that serializing OxyPlot classes is a good idea (e.g. #112). Why aren't you serializing your domain or data model?

objorke commented 8 years ago

I agree with @tibel - we don't want to support serialization of the plot model.

staeff777 commented 8 years ago

I'm working on the same project as @Scorpion226 and although I respect your design decisions, I cannot follow your argumentation, yet. If #112 was excepted and you are now using a Model instead of a ViewModel philosophy, isn't a model the correct pattern to be persisted?

Why aren't you serializing your domain or data model?

We use OxyPlot to visualize many different types of data. We process them using maps and filters and have different data types for all use cases. We don't build a large application, but many script-like apps that answer a specific scientific question. I'd like to persist the state of one chart to be able to view and annotate the results later and to be independent from our large data sources. Now we have two options: Create an own persistence layer that maps each of OxyPlots data model objects into serializable objects or store the data model directly. We prefer the second option, because this saves us from a lot of maintenance work especially on the long term, when your api evolves. I know that it's easier for us because we are trying to let you do the work, however I think that other users might find persistence useful, too.

Edit: Just to clarify: I'ts not about getting you to do the work. It's about the design decision. We also could use fork/pull request to implement a solution.

objorke commented 8 years ago

I think this is mostly a decision based on future maintenance concerns - about keeping backwards compatibility of the serializable model. It should not be difficult to make the model serializable, but this will make it more difficult to do refactoring of the model in the future. If we should support serialization we also need to cover serialization and deserialization of all properties in all annotations, axes, series and legends with unit tests - that's a big task, I think.

tibel commented 8 years ago

Some related info on serialization: https://twitter.com/terrajobst/status/662690170425094145

staeff777 commented 8 years ago

An alternative would be to support third party serialization libraries like protobuf. Most of them only require a parameterless constructor in each data model class. The serialisation then can be achieved independent to this project, however still use original data model.

objorke commented 8 years ago

I think all annotations, axes and series have parameterless constructors. Value types (like DataPoint) and types for "items" (e.g. ScatterPoint) are immutable and should not have parameterless constructors. The serializer must be configured to handle these types. When this is done, I think the model should be serializable - but I have not tested this myself.

Note that we will not guarantee backwards compatibility for the model.

What part of the PlotModel is not serializable? Can someone create an example?

objorke commented 8 years ago

Parameterless constructors everywhere is a good idea, I think. Which classes are not working? But the value types should be immutable and should not have parameterless constructors. But keeping a backwards compatible data structure is not something we should support here.