Upgrade to dotnet 8.0.302

[driekus77]

Hi everyone,

Don't know if you are willing to upgrade? But if so I found some time to upgrade this ProjNET to newest dotnet versions.

The lib project now targets netstandard2.0 and net8.0.
The other 2 projects (Benchmark and UnitTests) target net6.0 and net8.0

Tested on Mac M3 in JetBrains Rider: Unit- and Benchmark tests are working.
Benchmark results:

// * Summary *

BenchmarkDotNet=v0.12.1, OS=macOS 14.5 (23F79) [Darwin 23.5.0]
Apple M3 Pro, 1 CPU, 12 logical and 12 physical cores
.NET Core SDK=8.0.302
  [Host]     : .NET Core 8.0.6 (CoreCLR 8.0.624.26715, CoreFX 8.0.624.26715), Arm64 RyuJIT
  DefaultJob : .NET Core 8.0.6 (CoreCLR 8.0.624.26715, CoreFX 8.0.624.26715), Arm64 RyuJIT


|            Method |     Mean |     Error |    StdDev |   Median |
|------------------ |---------:|----------:|----------:|---------:|
|       SoAOneByOne | 6.351 ms | 0.1039 ms | 0.0867 ms | 6.314 ms |
|        SoABatched | 5.611 ms | 0.0895 ms | 0.0837 ms | 5.573 ms |
|  TightAoSOneByOne | 6.324 ms | 0.1173 ms | 0.1097 ms | 6.420 ms |
|   TightAoSBatched | 5.324 ms | 0.0169 ms | 0.0141 ms | 5.326 ms |
| LooserAoSOneByOne | 6.423 ms | 0.0271 ms | 0.0212 ms | 6.428 ms |
|  LooserAoSBatched | 5.965 ms | 0.0445 ms | 0.0417 ms | 5.988 ms |

// * Hints *
Outliers
  PerformanceTests.SoAOneByOne: Default       -> 2 outliers were removed (6.63 ms, 6.63 ms)
  PerformanceTests.TightAoSBatched: Default   -> 2 outliers were removed (5.44 ms, 5.45 ms)
  PerformanceTests.LooserAoSOneByOne: Default -> 3 outliers were removed, 4 outliers were detected (6.36 ms, 6.58 ms..6.59 ms)

// * Legends *
  Mean   : Arithmetic mean of all measurements
  Error  : Half of 99.9% confidence interval
  StdDev : Standard deviation of all measurements
  Median : Value separating the higher half of all measurements (50th percentile)
  1 ms   : 1 Millisecond (0.001 sec)

Fixed some errors and warnings:

• ProjectionParameterSet now has default constructor for JSon serialization. The old one is obsolete now because of security issue(s) with IFormatter/BinaryFormatter.
• Warnings about missing XML comment(s) fixed

[CLAassistant]

All committers have signed the CLA.

[airbreather]

The primary question to be answered: is it acceptable to drop support for the legacy .NET Framework by removing the netstandard2.0 target? Per #99, I don't think that I should answer this authoritatively, but I will give some context. Please take this comment in that spirit: it's going to sound negative, but I want to emphasize that I'm not contributing actively enough anymore for it to be appropriate for me to say things like "no".

This library dates back to at least as early as 2011 (and probably earlier, though I cannot confirm), whereas .NET Core (which is the one that later evolved to .NET) released version 1.0 in 2016, and the WPF application I develop at work had no chance of migrating away from .NET Framework until .NET Core 3.0 was released in 2019.

For that much history, I suspect that there are plenty of other .NET Framework-only applications which either cannot or will not migrate away. Looking at the download stats of the ProjNET package, over the past 6 weeks, the latest version was downloaded almost 600 times from clients whose versions are from the Visual Studio 2017 era or earlier. This is a lower-bound estimate of usage because of package caching.

So if it were my decision and I had the will to devote time to maintaining this in the future, then I would strive to maintain a netstandard2.0 target as long as Microsoft continues to support platforms that cannot use any cross-platform libraries any higher than that.

---

Scanning through this PR very quickly, it looks like there are no other changes outside the automated tests that actually use the new APIs and features present in netstandard2.1 which are not also in netstandard2.0. For this reason, unless I'm missing something, this PR does nothing but remove support for platforms (because netstandard2.0 is not a specific target, but rather a standard that is implemented by all platforms which also implement netstandard2.1 anyway). So even ignoring the compatibility issues, I fail to see the benefit of that part of the change?

Perhaps there's something specific that's coming later... but in my view, if you already have to restrict yourself to just the 37,118 APIs available in netstandard2.1 to support that set of platforms, then there doesn't seem to be much of a reason not to restrict it slightly further to the 32,638 APIs that let your library work with .NET Framework and UWP.

Especially now that Xamarin.Android and Xamarin.iOS are now out-of-support, this leaves just Mono and Unity as the two currently-supported targets which benefit from going to netstandard2.1 instead of net6.0.

---

I fully support updating target frameworks for those that target various other versions of .NET Core / .NET 5+.

[driekus77]

@airbreather Thanks for your quick reply.

I bumped back to netstandard2.0. But I'm trying to test this on my Windows laptop but debug branch is failing on:
CS0117 'Math' does not contain a definition for 'Atanh' ProjNET
Math.Atanh looks to be not supported in netstandard2.0.
I fixed above error in my features/upgrade_to_net8.0 branch with a pragma.

Is it an idea to leave the netstandard2.0 support as is but ADD multi targetframework support to net8.0?

Leaves the NUnit tests which seems to be very difficult to get them running for multiple target frameworks. E.g. on my mac its not working.... still investigating.

[driekus77]

Finally got something working:

Benchmark project now targets net6.0 and net8.0 same as the Unit Tests project.
The Unit tests where already targeting net6.0.
The Proj4NET lib still targets now netstandard2.0 and new net8.0.

[airbreather]

Math.Atanh looks to be not supported in netstandard2.0.

I see. I only looked at this PR, whereas the latest commit on develop is what actually broke the build, which is how I missed this.

Is it an idea to leave the netstandard2.0 support as is but ADD multi targetframework support to net8.0?

With libraries like this one, targeting the most restrictive target framework means that it can reach the most users, but also it means that the developers of that library have the smallest toolbox. It's a tradeoff, and I think that keeping a target for netstandard2.0 will not be extremely burdensome.

If the library consumers significantly benefit from something in net8.0 that isn't also present in netstandard2.0, then it could make sense to multi-target, but IMO it would need to be something very significant. In this case, I would personally use the (Math.Log(1 + x) - Math.Log(1 - x)) / 2 version unconditionally — probably with a comment like // Math.Atanh is not present in .NET Standard 2.0 — and skip adding the second target at this time. Implementing the same algorithm with different target-dependent code should be used as a last resort, or else it should come with compelling evidence that the benefits outweigh the drawbacks.

Math.Atanh does not strike me as compelling enough, especially since (to my eyes) it looks like AlbersProjection and HotineObliqueMercatorProjection both include calculations that could be improved with Math.Atanh.

Leaves the NUnit tests which seems to be very difficult to get them running for multiple target frameworks.

That would be one of the "drawbacks" I spoke of, earlier in this comment: in order to unit test a project that compiles different versions of the code for different target frameworks, then it is important to cover all different combinations with unit tests. Therefore, the tests should be multi-targeted. Therefore, at best, you hit the same issue I raised in microsoft/vstest#1013 years ago when we actually did this for NetTopologySuite (before we ultimately dropped support for platforms that could not implement netstandard2.0).

FWIW, some other drawbacks don't show themselves until much later, sometimes when it's too late to change course. There's one that applies to this case, and it was alluded to all the way back in dotnet/runtime#20322 (comment) when Math.Atanh was first proposed: Math.Atanh(x) will return different results than (Math.Log(1 + x) - Math.Log(1 - x)) / 2, even though those two formulations are supposedly mathematically equivalent, because these functions don't run at infinite precision. So if someone depends on this function in a net6.0 application (with their own unit tests for it), and then they upgrade to net8.0, they could see their unit tests start to fail because it was compiled differently.

[airbreather]

In this case, I would personally use the (Math.Log(1 + x) - Math.Log(1 - x)) / 2 version unconditionally

Quick clarification to this: I would probably actually use Math.Log((1 + x) / (1 - x)) / 2 instead of calling Math.Log twice.

[driekus77]

Thanks again for your extended reply @airbreather

I will fix the twice Math.Log this evening. (Now at work).

Concerning the multi target framework support the following:
I'm already busy working on a WKT1.0 parser replacement using a Parser combinator lib: https://github.com/benjamin-hodgson/Pidgin. This library is targeting net5.0+. This because of the Span<...> and Memory<...> etc. stuff.
And when I do have time left I will also add WKT2.0 support. A Parser combinator lib can easely be used to support multiple versions and reuse parts of code for backwards compatibility.
That is why I'm sugesting an extra target with the latest and greatest support.

I know that Multi Targeting frameworks can be throublesome but I don't know how to do WKT1/2 support needly without a Parser Combinator lib like Pidgin. Maybe its is doable to backport Pidgin to netstandard2.0 but I'm afraid they will not support it. The reason for me not to go on with current WKT code is because of its license and its code setup(Can't get it in my head ;-)).

To keep things short:
What about multi targeting: netstandard2.0 and net5.0?
The unit tests and benchmarktests then targeting netcoreapp3.1 and net5.0?

WKT2.0 is already requested: #83

[airbreather]

We already use spans via the System.Memory package, which should do the right thing for everything netcoreapp2.1 and above, including net5.0 and higher. If I am mistaken, can you please provide details for what is not working?

That is, specific code and/or a specific error message?

[driekus77]

We already use spans via the System.Memory package, which should do the right thing for everything netcoreapp2.1 and above, including net5.0 and higher. If I am mistaken, can you please provide details for what is not working?

That is, specific code and/or a specific error message?

Mmm Interesting! Didn't realize that. I'will go on and test it better this evening... I will come back on this. Thanks!

[driekus77]

I have some more info about why I did, maybe to much, changes.
When fixing the build warnings I stumbeled upon BinaryFormatter being deprecated. In order to fix this I broke the ApiCompat check. So I will withdraw this PR and create a new one.

The error when running the UnitTests with the NewtonsoftJson parser as object (De)Serializer:
Newtonsoft.Json.JsonSerializationException: Unable to find a default constructor to use for type ProjNet.CoordinateSystems.Projections.ProjectionParameterSet. Path 'latitude_of_origin', line 1, position 102.

When I add an extra Default constructor I come in, what i think, in trouble with the ApiCompat check(s):
Build error:

0>Microsoft.DotNet.ApiCompat.targets(82,5): Error  : CannotChangeAttribute : Attribute 'System.Runtime.CompilerServices.IteratorStateMachineAttribute' on 'ProjNet.CoordinateSystems.Projections.ProjectionParameterSet.ToProjectionParameter()' changed from '[IteratorStateMachineAttribute(typeof(ProjectionParameterSet.<ToProjectionParameter>d__4))]' in the contract to '[IteratorStateMachineAttribute(typeof(ProjectionParameterSet.<ToProjectionParameter>d__5))]' in the implementation.
0>Microsoft.DotNet.ApiCompat.targets(96,5): Error  : ApiCompat failed for 'C:\Users\AL17600\RiderProjects\ProjNet4GeoAPI\src\ProjNet\bin\Debug\netstandard2.0\ProjNET.dll'

So I close this one.

[driekus77]

We already use spans via the System.Memory package, which should do the right thing for everything netcoreapp2.1 and above, including net5.0 and higher. If I am mistaken, can you please provide details for what is not working?
That is, specific code and/or a specific error message?

Mmm Interesting! Didn't realize that. I'will go on and test it better this evening... I will come back on this. Thanks!

You are right: I managed to get the Fix PR #120 compiling against an older Pidgin version without targeting multiple frameworks. Unittests showing good.

For the newest version of Pidgin:
0>ProjNET.csproj: Error NU1202 : Package Pidgin 3.2.3 is not compatible with netstandard2.0 (.NETStandard,Version=v2.0). Package Pidgin 3.2.3 supports: net5.0 (.NETCoreApp,Version=v5.0)

Thanks again!

[airbreather]

I think I see what you're going for now, after taking a few minutes to really read and digest what you're saying here.

My own go-to for this is ANTLR, though I haven't yet found a way to integrate it into the build process without making me feel ill...