Skip to content
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

Adding a LoadColumnNameAttribute #4308

Merged
merged 4 commits into from Oct 10, 2019

Conversation

@tannergooding
Copy link
Member

commented Oct 7, 2019

This resolves #4195

@tannergooding tannergooding requested a review from dotnet/mlnet-core as a code owner Oct 7, 2019
@tannergooding

This comment has been minimized.

Copy link
Member Author

commented Oct 7, 2019

{
public int Label;

[LoadColumnName("SepalLength", "SepalWidth")]

This comment has been minimized.

Copy link
@CESARDELATORRE

CESARDELATORRE Oct 7, 2019

Contributor

Not sure I understand this. What if you have hundreds of columns to be loaded as a single vector. Do you need to specify all the column names? Can't you specify a column index range?

This comment has been minimized.

Copy link
@tannergooding

tannergooding Oct 8, 2019

Author Member

What would it mean to have a name and range? Why not just use the more efficient index based LoadColumn attribute at that point?

This comment has been minimized.

Copy link
@CESARDELATORRE

CESARDELATORRE Oct 8, 2019

Contributor

Sorry, that's what I mean, to use the LoadColumn attribute for those cases. I was thinking about the LoadColumn attribute. I didn't realize that here you are using LoadColumnName, not the same attribute, ok. ;)

@eerhardt

This comment has been minimized.

Copy link
Member

commented Oct 8, 2019

        /// Source index range(s) of the column.

This comment is now a bit out-of-date.

Source index range(s) of the column.

Range can now be more than just index ranges. #Closed


Refers to: src/Microsoft.ML.Experimental/DataLoadSave/Database/DatabaseLoader.cs:240 in 90316ec. [](commit_id = 90316ec, deletion_comment = False)

@eerhardt

This comment has been minimized.

Copy link
Member

commented Oct 8, 2019

    /// Specifies the range of indices of input columns that should be mapped to an output column.

This comment is now a bit out-of-date.

the range of indices of input columns

Range can now be more than just indices. #Closed


Refers to: src/Microsoft.ML.Experimental/DataLoadSave/Database/DatabaseLoader.cs:253 in 90316ec. [](commit_id = 90316ec, deletion_comment = False)

@codecov

This comment has been minimized.

Copy link

commented Oct 8, 2019

Codecov Report

Merging #4308 into master will decrease coverage by 0.01%.
The diff coverage is 43.07%.

@@            Coverage Diff             @@
##           master    #4308      +/-   ##
==========================================
- Coverage   74.57%   74.56%   -0.02%     
==========================================
  Files         878      879       +1     
  Lines      154277   154524     +247     
  Branches    16874    16883       +9     
==========================================
+ Hits       115058   115220     +162     
- Misses      34472    34553      +81     
- Partials     4747     4751       +4
Flag Coverage Δ
#Debug 74.56% <43.07%> (-0.02%) ⬇️
#production 70.14% <29.7%> (-0.03%) ⬇️
#test 89.54% <89.65%> (ø) ⬆️
Impacted Files Coverage Δ
...l/DataLoadSave/Database/LoadColumnNameAttribute.cs 100% <100%> (ø)
...perimental/DataLoadSave/Database/DatabaseLoader.cs 52.39% <67.27%> (+3.8%) ⬆️
test/Microsoft.ML.Tests/DatabaseLoaderTests.cs 90.9% <89.65%> (-0.68%) ⬇️
...ntal/DataLoadSave/Database/DatabaseLoaderCursor.cs 22.62% <9.48%> (-2.19%) ⬇️
....ML.AutoML/PipelineSuggesters/PipelineSuggester.cs 79.83% <0%> (-3.37%) ⬇️
src/Microsoft.ML.Maml/MAML.cs 24.75% <0%> (-1.46%) ⬇️
src/Microsoft.ML.Transforms/Text/LdaTransform.cs 84.44% <0%> (-0.16%) ⬇️
...c/Microsoft.ML.Dnn/ImageClassificationTransform.cs 87.42% <0%> (-0.02%) ⬇️
test/Microsoft.ML.Functional.Tests/Evaluation.cs 100% <0%> (ø) ⬆️
... and 9 more
Copy link
Member

left a comment

LGTM. Thanks!

@tannergooding tannergooding merged commit 60b1583 into dotnet:master Oct 10, 2019
19 checks passed
19 checks passed
MachineLearning-CI Build #20191010.2 succeeded
Details
MachineLearning-CI (Centos_x64_NetCoreApp30 Debug_Build) Centos_x64_NetCoreApp30 Debug_Build succeeded
Details
MachineLearning-CI (Centos_x64_NetCoreApp30 Release_Build) Centos_x64_NetCoreApp30 Release_Build succeeded
Details
MachineLearning-CI (MacOS_x64_NetCoreApp21 Debug_Build) MacOS_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (MacOS_x64_NetCoreApp21 Release_Build) MacOS_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Ubuntu_x64_NetCoreApp21 Debug_Build) Ubuntu_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Ubuntu_x64_NetCoreApp21 Release_Build) Ubuntu_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp21 Debug_Build) Windows_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp21 Release_Build) Windows_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp30 Debug_Build) Windows_x64_NetCoreApp30 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp30 Release_Build) Windows_x64_NetCoreApp30 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetFx461 Debug_Build) Windows_x64_NetFx461 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetFx461 Release_Build) Windows_x64_NetFx461 Release_Build succeeded
Details
MachineLearning-CI (Windows_x86_NetCoreApp21 Debug_Build) Windows_x86_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x86_NetCoreApp21 Release_Build) Windows_x86_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CodeCoverage Build #20191010.2 had test failures
Details
MachineLearning-CodeCoverage (Windows_x64 Build_Debug) Windows_x64 Build_Debug succeeded
Details
WIP Ready for review
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.