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

Refactor Validation class #69

Open
IroNEDR opened this issue Oct 27, 2024 · 1 comment
Open

Refactor Validation class #69

IroNEDR opened this issue Oct 27, 2024 · 1 comment
Assignees
Labels
enhancement New feature of request in progress This issue is being actively worked on

Comments

@IroNEDR
Copy link
Collaborator

IroNEDR commented Oct 27, 2024

Currently the PyNomaly library is using a validation class that offers several methods that print a warning and return a bool. Based on the returned bool value we either call some other function or even call sys.exit(). The following things should be changed:

  1. Rename methods: functions like _fit() should be renamed to indicate that it only checks something e.g. is_fit()
  2. Consider whether a validation class is even necessary, since all except for one of it's methods are static
  3. Throw proper, meaningful exceptions instead of calling sys.exit() if certain validation methods
@IroNEDR IroNEDR self-assigned this Oct 27, 2024
@IroNEDR IroNEDR added the in progress This issue is being actively worked on label Oct 27, 2024
@vc1492a
Copy link
Owner

vc1492a commented Oct 28, 2024

  1. Rename methods: functions like _fit() should be renamed to indicate that it only checks something e.g. is_fit()

Definitely.

  1. Consider whether a validation class is even necessary, since all except for one of it's methods are static

The original intent of this class was to provide helpful warning messages and errors and ensure that proper data types and formats are expected prior to execution. If we believe we can do that with item 3 (below) and without the whole Validation class, then I am for it.

Throw proper, meaningful exceptions instead of calling sys.exit() if certain validation methods.

Yes, let's do it 🚀

@vc1492a vc1492a added the enhancement New feature of request label Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature of request in progress This issue is being actively worked on
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants