ML.NET Notes #74
ML.NET Notes #74
Conversation
|
Hi @terrajobst , thanks for posting this. In order to track the correspondence of work to these notes, I will mark as comments on the specific lines corresponding issues (or comments on issues, where appropriate), where they are posted. If you would prefer in future I provide these annotations in a different way, please let me know, thanks! |
|
|
||
| ## Notes | ||
|
|
||
| * `Column` can be a struct |
There was a problem hiding this comment.
Added as a comment here, to address this and some other commetns.
| way, but it's hard to make the much simpler as they would have to be | ||
| generic or the user would have to handle the type | ||
| * `IDataView` | ||
| - `ISchematized` is being removed |
| generic or the user would have to handle the type | ||
| * `IDataView` | ||
| - `ISchematized` is being removed | ||
| - Should be an abstract class |
| implementers can still return `null`. | ||
| - Either make it a property `long? Count` or make it | ||
| `bool TryGetCount(out long count)`. | ||
| - `GetRowCursor()` instead of taking `Func<int, bool>` make it `Predicate<Column>` |
| * `IDataView` | ||
| - `ISchematized` is being removed | ||
| - Should be an abstract class | ||
| - `GetRowCount()`. `lazy = true` means get if you know it, otherwise don't |
| which makes it easier to reason about. | ||
| - We should look into whether we should have an async version | ||
| - We want to get rid of the the `IRowCursorConsolidator` base | ||
| * `ICounted` |
There was a problem hiding this comment.
Many of the things like this related to IRowCursor I have bundled into a single issue 1532 since I felt like this refactoring is something worth considering collectively, rather than individual items, since many of them are related. This actually includes all the rest of the file up to the ColumnType item.
... Except for the UInt128 issue now that I see it.
| * `IRowToRowMapper` | ||
| - `Action disposer` should probably be replaced by making the right type | ||
| (`IRow`?) implement `IDisposable`. | ||
| - `Func<int, bool>` is hard to get right. Should this be `Predicate<Column>` |
There was a problem hiding this comment.
Suggested this as part of 1529, which I'd mentioned previously.
| as well? | ||
| * Can we remove `DataKind` entirely? If not, we should probably rename some of the | ||
| members, remove the overlap, and add an entry for `0`, like `Unknown` or `None`. | ||
| * `ColumnType` |
There was a problem hiding this comment.
Entered as 1533, with some expansion, please take a look and tell me what you think. This includes a solution to the DataKind problem that I think will work pretty well and at fairly little trouble to anyone...
These notes are mostly published to help us (to remeber what happened in the meeting) and the community (to see what's going on). Commenting here is a fine. We could also file work items and link them from the notes, notes but that might be overkill. So whatever helps you stay organized is The Correct Process ™️ 😄 |
| - `GetRowCursor()` instead of taking `Func<int, bool>` make it `Predicate<Column>` | ||
| which makes it easier to reason about. | ||
| - We should look into whether we should have an async version | ||
| - We want to get rid of the the `IRowCursorConsolidator` base |
There was a problem hiding this comment.
Almost forgot about this one. Entered as 1867.
| * `IRowCursor` extending `IRow` seems odd. Can these be collapsed? If not, can | ||
| they have an inheritance relationship when we're making them classes? | ||
| * `IRowToRowMapper` | ||
| - `Action disposer` should probably be replaced by making the right type |
There was a problem hiding this comment.
Questions about disposability of IRow (or now Row) were fixed in 1867 so this strangeness around disposing methods has been resolved, thanks!
No description provided.