Skip to content

Conversation

@tonyyang-svail
Copy link
Collaborator

@tonyyang-svail tonyyang-svail commented Sep 10, 2019

Fix #809

@tonyyang-svail tonyyang-svail force-pushed the add_intermediate_represenstation branch from 7b93975 to d52d55b Compare September 11, 2019 19:45
Copy link
Collaborator

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please add a function in type FeatureColumn interface{} to garantee the FeatureColumn types.

@tonyyang-svail tonyyang-svail merged commit b282995 into sql-machine-learning:develop Sep 13, 2019
@tonyyang-svail tonyyang-svail deleted the add_intermediate_represenstation branch September 13, 2019 01:54
@tonyyang-svail
Copy link
Collaborator Author

tonyyang-svail commented Sep 14, 2019

LGTM, please add a function in type FeatureColumn interface{} to garantee the FeatureColumn types.

Here is nice blog posting listing possible alternatives for sum types in Go. https://making.pusher.com/alternatives-to-sum-types-in-go/

I vote for alternativ#1, in our case, we can add a isFeatureColumn method for all intended feature column types. I like this solution's simplicity. What do you think? @typhoonzero

@typhoonzero
Copy link
Collaborator

@tonyyang-svail Agree! Seems nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Intermediate Representation] Add examples to feature column in FieldMeta

2 participants