mathnet / mathnet-spatial

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

Update Framework support for 4.7.1 #138

Open Jones-Adam opened 6 years ago

Jones-Adam commented 6 years ago

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 6 years ago

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 6 years ago

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

Jones-Adam commented 6 years ago

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 6 years ago

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

JohanLarsson commented 6 years ago

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 6 years ago

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 5 years ago

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