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

Replace enums to be extendable #151

Merged
merged 3 commits into from
May 23, 2024
Merged

Replace enums to be extendable #151

merged 3 commits into from
May 23, 2024

Conversation

swashko
Copy link
Collaborator

@swashko swashko commented May 23, 2024

Replace SkipCategories, IssueCode, DefaultModelFormats enums with extendable classes
Renames DefaultModelFormats to SupportedModelFormats

Comment on lines +8 to +20
class Property:
def __init__(self, name: str, value: Any) -> None:
self.name = name
self.value = value


class SupportedModelFormats:
TENSORFLOW = Property("TENSORFLOW", "tensorflow")
KERAS_H5 = Property("KERAS_H5", "keras_h5")
KERAS = Property("KERAS", "keras")
NUMPY = Property("NUMPY", "numpy")
PYTORCH = Property("PYTORCH", "pytorch")
PICKLE = Property("PICKLE", "pickle")
Copy link
Contributor

@CandiedCode CandiedCode May 23, 2024

Choose a reason for hiding this comment

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

Enum already supports name and value.

I'd recommend adding support for str, if you want to enable additional behavior like f string support.

Screenshot 2024-05-23 at 4 53 26 PM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replacing the enum since it's not extendable, in the current enum state other applications adding their own scanners can't add additional model formats or issue codes

Comment on lines +9 to +15
class SkipCategories:
SCAN_NOT_SUPPORTED = Property("SCAN_NOT_SUPPORTED", 1)
BAD_ZIP = Property("BAD_ZIP", 2)
MODEL_CONFIG = Property("MODEL_CONFIG", 3)
H5_DATA = Property("H5_DATA", 4)
NOT_IMPLEMENTED = Property("NOT_IMPLEMENTED", 5)
MAGIC_NUMBER = Property("MAGIC_NUMBER", 6)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would IntEnum be better here?

Screenshot 2024-05-23 at 5 03 02 PM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same problem with not being extendable

@swashko swashko merged commit bc2bf6b into main May 23, 2024
8 checks passed
@swashko swashko deleted the replace-enums branch May 23, 2024 22:55
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.

3 participants