mathnet/mathnet-spatial

Update Framework support for 4.7.1

Jones-Adam opened this issue · 7 comments

Jones-Adam commented

With the support for netstandard2.0 and netstandard1.3 for legacy cases I would like to raise the native framework version to 4.7.1 and switch to using c# 7.2 for the core library, the unittests and benchmarking project.

As users of lower versions of the framework can link in the .netstandard versions I cannot see this being an issue for anyone but if it is please chime in.

cdrnet commented
  • .Net Standard 2.0 is only usable from 4.6.1 and newer, with workarounds (otherwise 4.7.1 and newer)
  • .Net Standard 1.3 is only usable from 4.6 and newer and is a very bad experience to use from .Net Framework dependency and deployment wise.

Hence, .Net Standard cannot actually cover any legacy cases.

Can't we switch to C# 7.2 without dropping direct support for older frameworks?

cdrnet commented

Or would we want to leverage new APIs only available in 4.7.1?

Jones-Adam commented

So far, the features I'd like to make use of are:
Tuples - 4.7
ReadOnlyCollection - 4.6
AggressiveInline attribute - 4.5
ISerializationSurrogateProvider - 4.7.1
Span - 4.7.1 (possibly - still evaluating its suitability for mesh, polyline and polygon implementations)

so yes I would love to move to 4.7.1 but I understand your concern. Currently 4.5.2 is the oldest framework version still under mainstream ms support (until October this year) so I would request that we at least move to that, although 4.6 would be much preferable due to the collection changes.

And yes we should go to c#7.2 regardless if only for the improved struct support (which as far as I can tell does not require framework changes)

cdrnet commented

@JohanLarsson are you aware of any users targeting .Net Framework < 4.6? What about < 4.7.1?

JohanLarsson commented

We use older versions in some projects but they are legacy so not a problem, if they are updated we will bump version for them. Unity is stuck on some really old version net35 IIRC, I'm not aware of any Unity users but then I'm not aware of any users.
Maybe we should try to multitarget.
We could for example create compatibility shims:

#if net35
namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Constructor | AttributeTargets.Method, Inherited = false)]
    public sealed class MethodImplAttribute : Attribute
    {
        public MethodImplAttribute(MethodImplOptions methodImplOptions)
        {
        }
    }
}

namespace System.Reflection
{
  public enum MethodImplAttributes
  {
    AggressiveInlining = 256,
  }
}
#endif

Stubs to keep the compiler happy, not sure if this will work.

@AQuentin2033 Where do we need tuples and ReadOnlyCollection?

Jones-Adam commented

I wanted to use ReadOnlyCollection internally for Polygon, polyline2D/3D
For Tuple there were a few places where the method signatures would be nicer with 2 or 3 element returns, a couple internal and a couple public from what I recall.

Regarding Unity - legacy unity builds against .net3.5 by default (although since 2017.1 you can choose .net4.6 and from 2018.1 the beta seems to let you choose .netstandard2.0). According to the current (and previous) nuget package for Spatial .net 4.0 is the minimum, so that would imply there cannot be any legacy unity users...

I suppose we could use shims and conditional compilation and backport code to support multiple framework versions. Its a lot of work though and harder to maintain and test and if there is no users requiring/requesting it then the value is pretty low. Users would also have the option of simply staying on their current version if they wish, nothing says they have to upgrade if they are happy with the current code.

cdrnet commented

FYI: I've dropped support for older platforms and upgraded the target frameworks to .NET Framework 4.6.1 and .NET Standard 2.0.