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

Make array generic. #172

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make array generic. #172

wants to merge 2 commits into from

Conversation

esoma
Copy link

@esoma esoma commented Dec 5, 2021

I've created type stubs for pyglm (https://github.com/esoma/pyglm-typing). There is a runtime wart with them however, glm.array is not generic (as in typing.Generic) and my typing stubs are. It works for the most part where annotations are only statically assessed, but some static typing is inevitably also runtime, for example in the case of type aliases:

MyVectorArray = array[vec2] | array[vec3] | array[vec4]

This will fail at runtime since the array class doesn't follow the generic typing protocol.

Luckily this is fairly easy to add in 3.9+: https://docs.python.org/3/c-api/typehints.html

So I've gone ahead and done that in this PR with the understanding that this isn't perfect. PyGLM of course doesn't officially support typing so this is kind of hacky (and btw, if you'd like to support typing I'd be happy to discuss merging the stubs into this repo). Also it's only 3.9+, it's possible to support this in various ways for all of PyGLM's supported versions (3.7+ would require a custom implementation of Py_GenericAlias and 3.5+ would require a metaclass -- those seem a much larger lift).

@Zuzu-Typ
Copy link
Owner

Hey there,

sorry about the delay. I'm a bit busy at the moment.
I've been looking into ways of implementing this in a way that would be supported by Python 3.6+, but haven't been able to do so yet.
It will likely take until late January 2022 for me to have enough time to completely get this to work.
Alternatively I could push a version with only the existing changes.
I haven't quite made up my mind yet.

Anyway, thank you for your work!

@esoma
Copy link
Author

esoma commented Dec 14, 2021

Sounds good. I neglected to mention there are ways around this without support from pyglm proper (using strings), but it's just a bit ugly. On the other hand it certainly feels bad to me to introduce a version check in a code base that is otherwise consistent in its features across all supported versions, so I certainly wouldn't feel devastated if it's rejected on those grounds 😉.

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

2 participants