Fix incorrect DataFrame min max computation with NULL

[asmirnov82]

Fixes #6733

[codecov]

Codecov Report

Merging #6734 (c28e1f0) into main (184e661) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6734      +/-   ##
==========================================
+ Coverage   68.87%   68.89%   +0.02%     
==========================================
  Files        1216     1216              
  Lines      250915   251125     +210     
  Branches    26259    26311      +52     
==========================================
+ Hits       172825   173021     +196     
+ Misses      71265    71242      -23     
- Partials     6825     6862      +37     

Flag	Coverage Δ
Debug	68.89% <100.00%> (+0.02%)	⬆️
production	63.39% <100.00%> (+0.02%)	⬆️
test	88.91% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...a.Analysis/PrimitiveDataFrameColumnComputations.cs	50.37% <ø> (+0.56%)	⬆️
src/Microsoft.Data.Analysis/DateTimeComputation.cs	39.16% <100.00%> (+2.64%)	⬆️
...icrosoft.Data.Analysis/PrimitiveColumnContainer.cs	83.04% <100.00%> (ø)
....Analysis/PrimitiveDataFrameColumn.Computations.cs	77.77% <100.00%> (ø)
...oft.Data.Analysis/PrimitiveDataFrameColumn.Sort.cs	87.34% <100.00%> (ø)
...icrosoft.Data.Analysis/PrimitiveDataFrameColumn.cs	81.37% <100.00%> (ø)
...st/Microsoft.Data.Analysis.Tests/DataFrameTests.cs	99.27% <100.00%> (+0.01%)	⬆️

... and 10 files with indirect coverage changes

[JakeRadMSFT]

Thanks @asmirnov82! Can you merge this into https://github.com/JakeRadMSFT/machinelearning/tree/u/jakerad/generic-math as well?

It's possible this was fixed by other changes in the generic math branch. Can you make sure it meets your needs/add/update tests?

[JakeRadMSFT]

I'd like the null check to stay consistent everywhere. We can analyze perf later ... and see if in-lining would help.

[asmirnov82]

Thanks @asmirnov82! Can you merge this into https://github.com/JakeRadMSFT/machinelearning/tree/u/jakerad/generic-math as well?

It's possible this was fixed by other changes in the generic math branch. Can you make sure it meets your needs/add/update tests?

This wasn't fixed by the latest changes in generic-math branch. I merged my changes, however this requires additional work to change new generic implementation (CalculateReduction method), until changes are done 2 new unit tests fail (TestIntComputations_MaxMin_WithNulls and TestIntComputations_MaxMin_OnEmptyColumn)

[JakeRadMSFT]

Thanks @asmirnov82! Can you merge this into https://github.com/JakeRadMSFT/machinelearning/tree/u/jakerad/generic-math as well?
It's possible this was fixed by other changes in the generic math branch. Can you make sure it meets your needs/add/update tests?

This wasn't fixed by the latest changes in generic-math branch. I merged my changes, however this requires additional work to change new generic implementation (CalculateReduction method), until changes are done 2 new unit tests fail (TestIntComputations_MaxMin_WithNulls and TestIntComputations_MaxMin_OnEmptyColumn)

Awesome @asmirnov82 - I've made those changes and merged. Can you take a look at them when you have a chance?

Changes for Reduction method
JakeRadMSFT@2856d3a

If you're up for it - I'd love for you to review my generic math PR once I have the .NET 8 branch setup.

It should be ready for review after next week.

[JakeRadMSFT]

Merged into Generic Math branch!

[JakeRadMSFT]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).