Skip to content

Clean dataframe math #6709

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 3 commits into from
Jun 5, 2023
Merged

Clean dataframe math #6709

merged 3 commits into from
Jun 5, 2023

Conversation

asmirnov82
Copy link
Contributor

  1. Remove redundant code from PrimitiveDataFrameColumn.BinaryOperations and PrimitiveDataFrameColumnArithmetic to make it more readable
  2. Remove extra cast to slightly improve performance in PrimitiveDataFrameColumn.BinaryOperations
  3. Remove converting to mutable buffers (involves memory coping) for operations where it's not necessary to increase performance in PrimitiveDataFrameColumnArithmetic

This is preliminary work for #6110

@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Merging #6709 (1ba1342) into main (f93ab25) will increase coverage by 0.07%.
The diff coverage is 55.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6709      +/-   ##
==========================================
+ Coverage   68.77%   68.85%   +0.07%     
==========================================
  Files        1215     1215              
  Lines      251729   250697    -1032     
  Branches    26250    26256       +6     
==========================================
- Hits       173136   172607     -529     
+ Misses      71777    71276     -501     
+ Partials     6816     6814       -2     
Flag Coverage Δ
Debug 68.85% <55.10%> (+0.07%) ⬆️
production 63.35% <55.10%> (+0.06%) ⬆️
test 88.88% <ø> (ø)

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

Impacted Files Coverage Δ
...ata.Analysis/PrimitiveDataFrameColumnArithmetic.cs 48.46% <ø> (-0.53%) ⬇️
...lysis/PrimitiveDataFrameColumn.BinaryOperations.cs 41.46% <55.10%> (ø)

... and 8 files with indirect coverage changes

@michaelgsharp
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@michaelgsharp michaelgsharp left a comment

Choose a reason for hiding this comment

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

I think a lot of this will be fixed by the GenericMath rewrite that @JakeRadMSFT is doing, but since we don't know when that will end up going in I'm going to go ahead and get this in for now so we can get the benefit of it asap.

@michaelgsharp michaelgsharp merged commit d18676b into dotnet:main Jun 5, 2023
@asmirnov82 asmirnov82 deleted the clean_dataframe_math branch June 22, 2023 08:26
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants