FlorianRhiem / pyGLFW

Python bindings for GLFW
MIT License
233 stars 36 forks source link

Add camelCase (and GLFW_ prefixes) #18

Closed Zuzu-Typ closed 6 years ago

Zuzu-Typ commented 6 years ago

Hi Florian,

I just did a fork of pyGLFW where I switched all variable names from lowercase_withunderscores to camelCase (or 'mixedCase') and added the 'GLFW' prefixes to the macros. Even though this is not the recommended PEP8 way, in this case the camelCase approach is very helpful when you want to port C++ code to Python.

That got me thinking...

So. long story short: I would like to ask you to add support for camelCase (and the GLFW_ prefixes) alongside lowercase_with_underscores. It would only have benefits - existing code still works, porting C++ code can be done a lot quicker and pyGLFW would fit the naming scheme of the library it's most likely accompanied by -- PyOpenGL (which also uses camelCase).

Naturally, the user would have to make a from import instead. from glfw import *

Please let me know what you think about the idea.

Thanks for your consideration and best regards, --Zuzu_Typ--

FlorianRhiem commented 6 years ago

Hello,

personally I prefer the pythonic style, for example using namespaces instead of prefixes, which is why I wrote the wrapper like this, but I definitely see your point about minimizing the differences to GLFW C code. Ideally the users could choose the variant they prefer or even mix both if they really want to.

I do not want to break anyone's code with a package upgrade, so I don't think using __all__ for this is the way to go, but instead perhaps a glfw.c module (or glfw.GLFW similarly to OpenGL.GL) to indicate that the user wants the C style?

To reduce the amount of duplicated code, it would make sense for one wrapper to mostly use the other. Perhaps most of the style-changes could then be automated, for example every member starting with a GLFW_ prefix in the C-like wrapper can be exposed without the prefix in the pythonic wrapper.

What's your opinion on the changes that aren't just code style? The one that, to me, clearly doesn't fit into a C-like wrapper would be the normalization of the gamma ramp. The wrapping of structs I'm not sure about how to make them the most comfortable for someone coming from C or C++, but now that I'm thinking about at it, perhaps returning named tuples instead of plain tuples would be the best for both wrappers? When it comes to creating the structs, C and C++ users simply need to squint hard enough to make parentheses look like braces. ;)

For the other changes, I feel like the C-like wrapper would benefit from having them as they are now:

What are your thoughts on doing it this way?

Best regards, Florian

Zuzu-Typ commented 6 years ago

Hi again.

I would like to point out that I'm not too familiar with the entirety of the GLFW library (normally I use it to create a window and capture events).

You can register my vote for glfw.GLFW :)

I'm not really affected by any of the other changes you've made, however I don't really get why you decided to force normalization of the gamma ramp -- sure there is no unsigned short in Python where the cap would be reasonable, but you could also just do a few quick type checks (or check wether or not the number is between 0.0 and 1.0 ) or add a macro (e.g. GLFW_NORMALIZE_GAMMA_RAMP_VALUES=False)

Best regards, --Zuzu_Typ--

FlorianRhiem commented 6 years ago

Yeah, glfw.GLFW is a bit redundant, but most users will also be or become familiar with OpenGL.GL, so that should be fine.

I think that to someone used to Python (and not used to C/C++), the range of 0 to 1 will feel more intuitive, but I like the idea of making it optional with a NORMALIZE_GAMMA_RAMP setting. With True as default there'll be no change to current users, but those who decide to use integers can switch the normalization of. I'm going to add that and the namedtuples today.

FlorianRhiem commented 6 years ago

As said in the pull request, the automatic name conversion at runtime hinders code analysis (at least by PyCharm, but most likely by most tools), so instead I created a small script to generate glfw/GLFW.py as an import-based wrapper around the pythonic API. I've pushed the commit to feature-clike-wrapper.

Please go ahead and give it a try (either install an sdist from that branch or simply download and add GLFW.py to the glfw package) and let me know if there's anything that should be changed or added.

FlorianRhiem commented 6 years ago

@Zuzu-Typ Did you have time to try out the C-style wrapper?

FlorianRhiem commented 6 years ago

Since there hasn't been any further feedback, I went ahead and released a new version containing the C-like wrapper.