dotnet / machinelearning

ML.NET is an open source and cross-platform machine learning framework for .NET.
https://dot.net/ml
MIT License
8.91k stars 1.86k forks source link

Fix inconsistency in DataFrameColumns Clone API implementation #7100

Closed asmirnov82 closed 2 months ago

asmirnov82 commented 3 months ago

There are no breaking changes on Public API, mosty only internal implementation is changed

Fixes #7092

asmirnov82 commented 3 months ago

@JakeRadMSFT

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 78.75000% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 68.46%. Comparing base (79b5475) to head (69e2374).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #7100 +/- ## ========================================== - Coverage 68.46% 68.46% -0.01% ========================================== Files 1263 1263 Lines 254956 254945 -11 Branches 26352 26353 +1 ========================================== - Hits 174552 174542 -10 + Misses 73698 73697 -1 Partials 6706 6706 ``` | [Flag](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | Coverage Δ | | |---|---|---| | [Debug](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `68.46% <78.75%> (-0.01%)` | :arrow_down: | | [production](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `62.85% <78.75%> (-0.01%)` | :arrow_down: | | [test](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `88.58% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | Coverage Δ | | |---|---|---| | [src/Microsoft.Data.Analysis/DataFrameColumn.cs](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100?src=pr&el=tree&filepath=src%2FMicrosoft.Data.Analysis%2FDataFrameColumn.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#diff-c3JjL01pY3Jvc29mdC5EYXRhLkFuYWx5c2lzL0RhdGFGcmFtZUNvbHVtbi5jcw==) | `63.17% <100.00%> (+0.33%)` | :arrow_up: | | [...Analysis/DataFrameColumns/StringDataFrameColumn.cs](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100?src=pr&el=tree&filepath=src%2FMicrosoft.Data.Analysis%2FDataFrameColumns%2FStringDataFrameColumn.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#diff-c3JjL01pY3Jvc29mdC5EYXRhLkFuYWx5c2lzL0RhdGFGcmFtZUNvbHVtbnMvU3RyaW5nRGF0YUZyYW1lQ29sdW1uLmNz) | `71.17% <100.00%> (-0.44%)` | :arrow_down: | | [...icrosoft.Data.Analysis/PrimitiveDataFrameColumn.cs](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100?src=pr&el=tree&filepath=src%2FMicrosoft.Data.Analysis%2FPrimitiveDataFrameColumn.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#diff-c3JjL01pY3Jvc29mdC5EYXRhLkFuYWx5c2lzL1ByaW1pdGl2ZURhdGFGcmFtZUNvbHVtbi5jcw==) | `72.44% <78.94%> (-0.09%)` | :arrow_down: | | [...nalysis/DataFrameColumns/VBufferDataFrameColumn.cs](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100?src=pr&el=tree&filepath=src%2FMicrosoft.Data.Analysis%2FDataFrameColumns%2FVBufferDataFrameColumn.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#diff-c3JjL01pY3Jvc29mdC5EYXRhLkFuYWx5c2lzL0RhdGFGcmFtZUNvbHVtbnMvVkJ1ZmZlckRhdGFGcmFtZUNvbHVtbi5jcw==) | `44.60% <61.53%> (-1.18%)` | :arrow_down: | | [...sis/DataFrameColumns/ArrowStringDataFrameColumn.cs](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100?src=pr&el=tree&filepath=src%2FMicrosoft.Data.Analysis%2FDataFrameColumns%2FArrowStringDataFrameColumn.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#diff-c3JjL01pY3Jvc29mdC5EYXRhLkFuYWx5c2lzL0RhdGFGcmFtZUNvbHVtbnMvQXJyb3dTdHJpbmdEYXRhRnJhbWVDb2x1bW4uY3M=) | `63.17% <68.00%> (-0.38%)` | :arrow_down: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/dotnet/machinelearning/pull/7100/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet)
asmirnov82 commented 2 months ago

Looks good to me. Can you resolve the conflicts @asmirnov82? They might have come from me just merging in your other PR.

Thanks, Michael. Conflicts are resolved now