-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Extract IDataView into its own assembly and NuGet package #2220
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I should have commented on this originally. My initial implementation is just to move the types in the same namespace. Then the next change is to move namespaces. Since that is going to affect a huge number of files, I wanted to put that in its own PR for easy reviewing. I am also considering supporting more TFMs in the new library ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33c8cec
to
eb62d83
Compare
Codecov Report
@@ Coverage Diff @@
## master #2220 +/- ##
==========================================
- Coverage 69.78% 69.78% -0.01%
==========================================
Files 786 783 -3
Lines 143945 143438 -507
Branches 16648 16588 -60
==========================================
- Hits 100451 100094 -357
+ Misses 38951 38813 -138
+ Partials 4543 4531 -12
|
b2138d9
to
86a9ed2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels wrong to not have Contracts.Assert
and Check
on IDataView
... it's growing up... 😄 Thanks @eerhardt
Moving IDataView and related types into a new library:
Microsoft.Data.DataView
Fix #1860
I've split the changes into 3 commits for easy reviewing:
3 Get ML.NET building on the new assembly