mathnet / mathnet-spatial

Math.NET Spatial
http://spatial.mathdotnet.com
MIT License
376 stars 132 forks source link

Replace call to obsolete XmlExt by XmlWriteExt #181

Closed rsinnig closed 1 year ago

rsinnig commented 4 years ago

As discussed in cancelled pull request for fix in obsolete XmExt: Fix of the XML serialization issue (writing without InvariantCulture) by replacing all calls to obsolete extension method XmlExt.WriteAttribute by XmlWriteExt.WriteElement.

rsinnig commented 4 years ago

Failed test caused by different XML-formatting of obsolete XmlExt.WriteAttribute and new internal XmlWriteExt.WriteElementString While XmlExt.WriteAttribute is writing start tag, value, end tag (no new line and tabs inserted) the XmlWriteExt.WriteElementString is creating nice formatting with newline and tabs as long as XmlWriter is not explicitly configured to omit these.

From my perspective the XmlSerializer test needs to be modified.

rsinnig commented 4 years ago

The test procedure public static T XmlSerializerRoundTrip(T item, string expected) uses the extension methods from obsolete XmlExt. I would propose to add format provider fixed version of XmlExt.WriteAttribute into XmlWriterExt instead. This should fix the operational serialization issues as well as test procedures.

jkalias commented 1 year ago

@rsinnig @JohanLarsson any progress on this?

JohanLarsson commented 1 year ago

No progress

jkalias commented 1 year ago

Is this a legitimate concern/bug? Should we fix this? Apologies, but I am not really sure I understand what the problem is. If yes, can someone else from the team take of it?

JohanLarsson commented 1 year ago

I don't remember either, sorry.

jkalias commented 1 year ago

Ok, fair enough. I propose we give this issue one month time, and if it is not addressed till then, we reject it. Agree? @rsinnig @JohanLarsson

jkalias commented 1 year ago

I think I understand now what the issue is, having worked in #208. I also think that WriteElement is a better approach.

jkalias commented 1 year ago

I have noticed that there is an existing branch open. Can you @JohanLarsson @Jones-Adam please have a look, that my commits in this PR are heading in the right direction?

The serialization branch is quite old, and not actively maintained.

I am trying to unify the XML serialization and data contract tests, it seems like a bit messy at the moment.