Implement `enum` constraint

danielfireman commented 7 years ago

The value of the field must exactly match a value in the enum array.

JS implementation (for reference)

cored commented 7 years ago

@danielfireman will work on this one

danielfireman commented 7 years ago

Nice!

danielfireman commented 7 years ago

Hi @cored, still planning to work on this issue? This is the last issue to achieve compliance with the basic spec features :smile:

cored commented 7 years ago

@danielfireman I was about to work on it, just now :-)

danielfireman commented 7 years ago

Awesome! Go for it! :)

Em 13 de out de 2017 19:24, "Rafael George" notifications@github.com escreveu:

@danielfireman https://github.com/danielfireman I was about to work on it, just now :-)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/frictionlessdata/tableschema-go/issues/48#issuecomment-336581657, or mute the thread https://github.com/notifications/unsubscribe-auth/AIiWQyc2IN04O3rTRrJc4_VvnmmbtsK7ks5sr-MhgaJpZM4Pp6oV .

cored commented 7 years ago

@danielfireman I don't see the Enum type define in the code base, so I assume that this PR needs to include it?

danielfireman commented 7 years ago

Exactly!

Em 13 de out de 2017 19:35, "Rafael George" notifications@github.com escreveu:

@danielfireman https://github.com/danielfireman I don't see the Enum type define in the code base, so I assume that this PR should include it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/frictionlessdata/tableschema-go/issues/48#issuecomment-336583131, or mute the thread https://github.com/notifications/unsubscribe-auth/AIiWQ557YQa0a8uIKFLsHdyKriZ6okuqks5sr-WbgaJpZM4Pp6oV .

cored commented 7 years ago

@danielfireman I'm a little bit confused on how to represent an enum type at the Field struct level. I was considering this structure <name>: <value>, however, I don't know if that would be a good approach.

danielfireman commented 7 years ago

From the contraints description:

All constraints MUST be tested against the logical representation of data

So, we need to:

decode the enum items to the specific field type
- That can be done only once when creating the field (due to performance reasons).
- Following the pattern constraint, we could it here

That said, I would recommend using the following approach:

Create a RawEnum []string field in schema.Constraints struct. It should have a json tag name "enum"
Create an Enum []interface{} field in schema.Constraints struct. It should have a json tag name "-"

With that you could decode the RawEnum into Enum field and use the latter as a common validation to Decode method.

How does that sound?

cored commented 7 years ago

@danielfireman I'm still a little bit confused on this one. I thought that the new enum type was needed however still begs the question what would be the JSON input? As far as that representation goes you have an array of strings with the values of the enum however where's the name of such enum type coming in the input? When I see the tests in the field_test.go I see that every single type has the input representation so in that case, I don't see how to do the decoding. I'll read the snippets that you sent and see if I can figure it out. Thanks

danielfireman commented 7 years ago

I thought that the new enum type was needed however still begs the question what would be the JSON input?

The JSON input would be of type string slice, within schema.Constraints. With that slice of strings in hand, you could use the decode* functions and convert from the string to the logical type of the field (regardless the type as there are already decoders for all types). My previous answer suggests that the enum field decode could be done when the schema.Field unmarshalling, preventing the enum decoding from unnecessarily happening every time a field.Decode happens.

With the enum decoded, you could only verify (using reflect.DeepEquals) when field.Decode is called.

I'll read the snippets that you sent and see if I can figure it out.

Thanks! Let me know if you have more questions!

danielfireman commented 6 years ago

Hi @cored .. Passing just to let you know that I am going to start to work on this today. We need to close this up to move towards feature-completeness. Thanks for your effort!

cored commented 6 years ago

Yes, go ahead. I got caught up with stuffs from work. I will read your approach either way because I was kinda blocked. Sorry for the delay on giving you an status for the change.

Thanks for jumping in,

On Sat, Nov 4, 2017 at 10:00 AM Daniel Fireman notifications@github.com wrote:

Hi @cored https://github.com/cored .. Passing just to let you know that I am going to start to work on this today. We need to close this up to move towards feature-completeness. Thanks for your effort!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/frictionlessdata/tableschema-go/issues/48#issuecomment-341899108, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAtx_orZNSEAYiBSO77FN1Z6C4dRePeks5szG4ZgaJpZM4Pp6oV .

danielfireman commented 6 years ago

Fixed by 4d678fdb94b9c105cbee861a755fef2c1ad69b3b

frictionlessdata / tableschema-go

Implement `enum` constraint #48