Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Store] Add MinMaxLenValidator and RegexValidator #1702

Merged
merged 10 commits into from Mar 21, 2022
Merged

[Feature Store] Add MinMaxLenValidator and RegexValidator #1702

merged 10 commits into from Mar 21, 2022

Conversation

george0st
Copy link
Collaborator

@george0st george0st commented Feb 1, 2022

Add MinMaxLenValidator for Feature, including add system test and add RegexValidator (validation based on regular expression). It is very similar such as existing MinMaxValidator for Feature.

Add MinMaxLenValidator for Feature
Validation of feature value based on regular expression
@george0st george0st changed the title [Feature store] Add MinMaxLenValidator [Feature store] Add MinMaxLenValidator and RegexValidator Feb 19, 2022
Update sample for lint
@george0st
Copy link
Collaborator Author

george0st commented Feb 19, 2022

@yaronha , ready for merge, many thanks

@yaronha
Copy link
Collaborator

yaronha commented Feb 24, 2022

@george0st thanks for the PR :) see my comments

mlrun/features.py Outdated Show resolved Hide resolved
mlrun/features.py Outdated Show resolved Hide resolved
mlrun/features.py Outdated Show resolved Hide resolved
Add validation for basic numeric types, ...
Create system test file for Feature validation and add first validator for feature types. It makes sense (if content is ok) to add here other validators such as MinMaxValidator, MinMaxLenValidator, RegexValidator, etc.
@george0st george0st requested a review from yaronha March 2, 2022 22:04
@george0st
Copy link
Collaborator Author

george0st commented Mar 14, 2022

@urihoenig , ready for merge (see previous approval from yaronha)

@george0st
Copy link
Collaborator Author

@Hedingber , ready for merge (see previous approval from yaronha)

mlrun/features.py Outdated Show resolved Hide resolved
mlrun/features.py Outdated Show resolved Hide resolved
mlrun/features.py Outdated Show resolved Hide resolved
mlrun/features.py Outdated Show resolved Hide resolved
mlrun/features.py Outdated Show resolved Hide resolved
mlrun/features.py Outdated Show resolved Hide resolved
Cover exceptions, add limited dump string and remove validation for BOOL and STRING type
Add note for BOOL type and add tests for STRING
@george0st
Copy link
Collaborator Author

@gtopper , resolved issues, ready for merge, many thanks for check.

Copy link
Collaborator

@gtopper gtopper left a comment

Choose a reason for hiding this comment

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

Looks good ✅

@Hedingber Hedingber changed the title [Feature store] Add MinMaxLenValidator and RegexValidator [Feature Store] Add MinMaxLenValidator and RegexValidator Mar 21, 2022
@Hedingber Hedingber merged commit 939deb7 into mlrun:development Mar 21, 2022
@Hedingber
Copy link
Contributor

@george0st sorry for the delay, merged 🎉

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.

None yet

4 participants