Making Schema implement ISchema explicitly #1894
Conversation
Zruty0
changed the title
| @@ -85,7 +85,7 @@ public static void FeatureContributionCalculationTransform_Regression() | |||
| var value = row.Features[featureOfInterest]; | |||
| var contribution = row.FeatureContributions[featureOfInterest]; | |||
| var percentContribution = 100 * contribution / row.Score; | |||
| var name = data.Schema.GetColumnName(featureOfInterest + 1); | |||
| var name = data.Schema[(int) (featureOfInterest + 1)].Name; | |||
There was a problem hiding this comment.
(int) [](start = 39, length = 5)
why cast to int if it's already int?
To catch overflow? #Resolved
There was a problem hiding this comment.
I have no idea, this is Resharper doing the work for me. I believe you are right, it's for the overflow.
In reply to: 242359826 [](ancestors = 242359826)
Zruty0
force-pushed
the
pr/more-schema-cleanup
branch
from
5ecf4ed to
6ed35f0
Compare
Zruty0
changed the title
|
You can start looking now. Again, biggest changes are in |
| @@ -310,38 +300,37 @@ private static Delegate GetMetadataGetterDelegate<TValue>(ISchema schema, int co | |||
| return getter; | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Legacy method to get the column index. | |||
| /// DO NOT USE: use <see cref="GetColumnOrNull"/> instead. | |||
There was a problem hiding this comment.
DO NOT USE [](start = 12, length = 10)
I'm guessing using actual Obsolete on this and other code is undesirable? This one also looks very different from the others -- this is [BestFriend] internal while the remainder are explicit, was that intentional, or just an artifact of the work being staged somehow? #Resolved
There was a problem hiding this comment.
It's 450+ uses, and they cannot be all fixed mechanically with Resharper. So I opted to internal-best-friend it. The rest of the ISchema will be removed, but this one, I think, may stay for longer.
Actually obsolete-ing is going to be quite disruptive to the code base either...
In reply to: 242417023 [](ancestors = 242417023)
I will know today. The 2 major blockers was Schema and ColumnBindingsBase. In reply to: 448109260 [](ancestors = 448109260) Refers to: src/Microsoft.ML.Core/Data/Schema.cs:25 in 5b17cc4. [](commit_id = 5b17cc4, deletion_comment = False) |
ghost
locked as resolved and limited conversation to collaborators
Addresses #1500
Made Schema implement ISchema explicitly. Removed all calls to the ISchema inteface when Schema is accessed, with the exception of TryGetColumnIndex, which was made internal.