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.

Sign up for GitHub

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

Jump to bottom

Field type to validate "free" text inputs #68

Open
AndreFCruz opened this issue May 23, 2022 · 2 comments · May be fixed by #69
Open

Field type to validate "free" text inputs #68

AndreFCruz opened this issue May 23, 2022 · 2 comments · May be fixed by #69
Assignees
@AndreFCruz
Labels
enhancement New feature or request

Comments

@AndreFCruz
Copy link

AndreFCruz commented May 23, 2022 •
edited by ghost
Loading

I'm using the FreeTextFieldType class for an input that takes the following form:

  • <input_name>=<sequence-of-numbers>
  • e.g., arg1=1,0.3,9.8,2

I'm using the free text field as this is the only current suitable option, but I think it would be useful to add an optional parameter to the class constructor with a regex validator string, so we can check whether the input is valid before spinning-up a new model train process.

I think this minor modification to FreeTextFieldType could be useful to more people; and in my use case I'd just implement the input above as: new FreeTextFieldType("", "^((\d+(\.\d*)?,)*(\d+(\.\d*)?))?$") (1st parameter is the default value)

Shall I implement this and send in a PR?

PS: another option would be creating an entirely new FieldType CSVNumericFieldType that would be more specific to my case and would require larger code changes, but would also work.
@ghost ghost assigned AndreFCruz May 24, 2022
@ghost ghost added the enhancement New feature or request label May 24, 2022
@ghost
Copy link

ghost commented May 24, 2022

It's an okay from me. Please remember to not introduce breaking change (e.g., overload the constructor instead of changing its signature)

@antssilva96
Copy link

I think that makes sense, the current validate method is doing nothing anyways 🤔

Make sure that if no validation is provided the result is still a Optional.empty() to preserve current behavior.

If you need any help, let me know!

@AndreFCruz AndreFCruz linked a pull request May 24, 2022 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AndreFCruz AndreFCruz

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Added a regex validator for FreeTextFieldType AndreFCruz/feedzai-openml
2 participants
@AndreFCruz @antssilva96