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

Imath - Remove Enumerated Class and replace with Enum Class #1609

Open
404Vector opened this issue Jan 5, 2024 · 1 comment
Open

Imath - Remove Enumerated Class and replace with Enum Class #1609

404Vector opened this issue Jan 5, 2024 · 1 comment
Labels
Enhancement A request for a change or enhancement.

Comments

@404Vector
Copy link

https://github.com/AcademySoftwareFoundation/openexr/blob/86d0fb09859951d1e51a889e4ff2b7b3baecf021/src/wrappers/python/Imath.py#L68C1-L68C1

In older versions of Python, enum classes did not exist, and developers had to create them themselves if necessary.

However, the enum class was added starting with Python 3.4 version.

Therefore, using a self-defined class instead of using Python's default class causes confusion to developers.

Additionally, the enum class already supports the functions directly implemented in Enumerated and its child classes (LineOrder, Compression, etc.).

  • repr, cmp, eq, names, etc.

Therefore, unless you absolutely need to support very old versions (python2 and python3 below 3.4), I think it is a good idea to replace it.

@meshula meshula added the Enhancement A request for a change or enhancement. label Jan 6, 2024
@cary-ilm
Copy link
Member

cary-ilm commented Jan 9, 2024

Our intention is to completely replace that Imath.py with a proper import of the bindings from the Imath project. The current contents are a remnant of a much earlier implementation that needs a serious update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A request for a change or enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants