Support for read-only properties #640
Conversation
Ivanidzo4ka
changed the title
| /// <returns>The generated schema definition.</returns> | ||
| public static SchemaDefinition Create(Type userType) | ||
| public static SchemaDefinition Create(Type userType, Direction direction) |
There was a problem hiding this comment.
direction [](start = 71, length = 9)
I guess it should be Both by default... #Resolved
| // This property is ignored because it has a private getter | ||
|
|
||
| [NoColumn] | ||
| // This property can be used as recipticle for dataview, but not as source for dataview. |
There was a problem hiding this comment.
recipticle [](start = 44, length = 10)
receptacle #Resolved
| @@ -978,5 +981,48 @@ public void RoundTripConversionWithArrayPropertiess() | |||
| Assert.True(!enumeratorNullable.MoveNext() && !originalNullalbleEnumerator.MoveNext()); | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| class ClassWithPrivateGetter | |||
There was a problem hiding this comment.
PrivateGetter [](start = 23, length = 13)
not 'private', just 'getter' #Resolved
src/Microsoft.ML.Api/MapTransform.cs
Outdated
| @@ -36,7 +36,7 @@ internal sealed class MapTransform<TSrc, TDst> : LambdaTransformBase, ITransform | |||
| private static string RegistrationName { get { return string.Format(RegistrationNameTemplate, typeof(TSrc).FullName, typeof(TDst).FullName); } } | |||
|
|
|||
| /// <summary> | |||
| /// Create a a map transform that is savable iff <paramref name="saveAction"/> and <paramref name="loadFunc"/> are | |||
| /// Create a a map transform that is savable if <paramref name="saveAction"/> and <paramref name="loadFunc"/> are | |||
There was a problem hiding this comment.
if [](start = 53, length = 2)
'iff' is a actual wordd. It meanss 'if and only if'. #Resolved
|
@TomFinley @yaeldekel @ganik @dsyme any one is willing to review? |
| { | ||
| Read = 0, | ||
| Write = 1, | ||
| Both = Read | Write |
There was a problem hiding this comment.
Both = Read | Write [](start = 12, length = 19)
Note that since Read is 0, and Write is 1, and 10|1==1 you cannot distinguish between Both and Write as they are identical, this is not right -- at least, so I think given how you're using this enum. I think you meant to do something like have read and write be 1 and 2 respectively. Also I think if you're doing this you're going to want to set the Flags attribute on the enum.
You could perhpas have just two things, since the utility of write-only properties is far from clear to me. #Resolved
There was a problem hiding this comment.
Hi @Ivanidzo4ka thanks for looking at this -- one thing that I mentioned that might have gotten lost in my comment, was using the Flags attribute, which is sort of a nice thing to put on enums intended to be used as bitflags.
In reply to: 212057212 [](ancestors = 212057212)
There was a problem hiding this comment.
Hi @Ivanidzo4ka , pretty close the only real problem I'm seeing is the whole enum. Also maybe you want to more clearly title your PR since the way it's titled now "sketch" strongly suggests it is not something that is finalized.
Ivanidzo4ka
changed the title
|
@TomFinley I think I address all your comments, can we check this in? |
|
@TomFinley can you please take a look at this PR :) |
ghost
locked as resolved and limited conversation to collaborators
Suggestion how to fix #631
This would let user pass partially visible properties, but in same time, use will have to mark some of them if he don't want them to be exposed.