Skip to content

Fix DataFrame Null Math #6661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 8, 2023

Conversation

JakeRadMSFT
Copy link
Contributor

@JakeRadMSFT JakeRadMSFT commented May 6, 2023

Update Math logic to filter out null instead of treating it as default.

Related Issue:

#6659

Fix math and update templates to match generated files.
Add Tests
Fix test
@@ -844,7 +844,7 @@ public void TestComputations()
Assert.Equal(100.0, df.Columns["Double"].Max());
Assert.Equal(-10.0f, df.Columns["Float"].Min());
Assert.Equal((uint)0, df.Columns["Uint"].Product());
Assert.Equal((ushort)140, df.Columns["Ushort"].Sum());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{Ushort: 0 1 3 6 10 null 16 23 31 40 }

0+1+3+6+10+16+23+31+40 = 130

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JakeRadMSFT is this how Pandas handles math with null data? We should match whatever they do.

Same issue?
@JakeRadMSFT JakeRadMSFT changed the title WIP - Fix DataFrame Null Math Fix DataFrame Null Math May 7, 2023
Test and Fix Median and Mean
@codecov
Copy link

codecov bot commented May 7, 2023

Codecov Report

Merging #6661 (55f01d3) into main (3986fcf) will increase coverage by 0.06%.
The diff coverage is 99.83%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6661      +/-   ##
==========================================
+ Coverage   68.59%   68.66%   +0.06%     
==========================================
  Files        1200     1201       +1     
  Lines      250326   250799     +473     
  Branches    26096    26192      +96     
==========================================
+ Hits       171701   172199     +498     
+ Misses      71809    71783      -26     
- Partials     6816     6817       +1     
Flag Coverage Δ
Debug 68.66% <99.83%> (+0.06%) ⬆️
production 63.15% <99.81%> (+0.08%) ⬆️
test 88.86% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...icrosoft.Data.Analysis/PrimitiveDataFrameColumn.cs 81.17% <50.00%> (+1.82%) ⬆️
...a.Analysis/PrimitiveDataFrameColumnComputations.cs 49.81% <100.00%> (+4.14%) ⬆️
...st/Microsoft.Data.Analysis.Tests/DataFrameTests.cs 99.24% <100.00%> (+<0.01%) ⬆️
...Tests/PrimitiveDataFrameColumnComputationsTests.cs 100.00% <100.00%> (ø)

... and 16 files with indirect coverage changes

Update Cumulative handling of nulls based on DataFrame documentation
@michaelgsharp michaelgsharp merged commit 4f0afc3 into dotnet:main May 8, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants