[Tiny & Simple Change] Example classes should be static #2548
Conversation
wschin
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2548 +/- ##
==========================================
- Coverage 71.35% 71.34% -0.02%
==========================================
Files 799 799
Lines 141496 141496
Branches 16120 16120
==========================================
- Hits 100971 100946 -25
- Misses 36062 36088 +26
+ Partials 4463 4462 -1
|
| @@ -6,12 +6,12 @@ | |||
|
|
|||
| namespace Microsoft.ML.Samples.Dynamic | |||
| { | |||
| class OnnxTransformExample | |||
| public static class OnnxTransformExample | |||
There was a problem hiding this comment.
(nit) you've dropped the Example suffix in other places, but not here.
| @@ -5,7 +5,7 @@ | |||
|
|
|||
| namespace Microsoft.ML.Samples.Dynamic | |||
| { | |||
| public class ValueMappingExample | |||
| public static partial class ValueMapping | |||
There was a problem hiding this comment.
partial [](start = 18, length = 7)
(nit) it doesn't have to be partial
| @@ -6,7 +6,7 @@ | |||
|
|
|||
| namespace Microsoft.ML.Samples.Dynamic | |||
| { | |||
| public partial class TransformSamples | |||
| public static partial class TransformSamples | |||
There was a problem hiding this comment.
TransformSamples [](start = 32, length = 16)
All timeseries example slightly different from our current samples.
They share same class and introduce data classes inside it, instead of using sample utils.
But I'm not sure they transformation to how we see samples should be done in your PR.
|
|
||
| namespace Microsoft.ML.Samples.Dynamic | ||
| { | ||
| public class PriorTrainerSample | ||
| public class PriorTrainer |
There was a problem hiding this comment.
PriorTrainer [](start = 17, length = 12)
if you decide to update PR for nit comments, can you move this class and RandomTrainer to BinaryClassification folder? (If you decide to do it, don't forget to update catalog comments which looks on this file)
|
Thank you for doing this @wschin ! |
ghost
locked as resolved and limited conversation to collaborators
Polish DYNAMIC examples to fix #2549. This change is inspired by @TomFinley's good comment to #2506 and I sincerely want to address that.
Example()usingstatements.Please note that this PR only focuses on dynamic examples'
staticproblem. I don't have much bandwidth to take care other cases.