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

mixedCase naming convention #179

Closed
mottosso opened this issue Jan 30, 2017 · 8 comments
Closed

mixedCase naming convention #179

mottosso opened this issue Jan 30, 2017 · 8 comments

Comments

@mottosso
Copy link
Owner

Currently, load_ui is snake_case but Qt and it's bindings are all mixedCase.

I originally chose this convention so as to conform with PEP8 but in retrospect it doesn't seem appropriate as it intermixed two conventions in one library.

This issue then is about conforming new members of QtCompat to members of other submodules, e.g. QtCore and QtGui.

E.g. loadUi

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Jan 31, 2017

Hm. I thought it was good not to confuse load_ui with e.g. PyQt4.uic.loadUi as the behaviour is not 100% identical.

I like the snake case :) primarily because that makes it easy to spot what's part of Qt.py and what's not. But it's not something I would want to fight over. You decide :)

@mottosso
Copy link
Owner Author

mottosso commented Jan 31, 2017 via email

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Feb 1, 2017

Yes, I see the dilemma.

I know what the pep says, but let's not get too tangled up in that. The pep is really just guidelines in this case. We're going past that here and need to define our own rules.

Right now I think it would be very weird to rename setSectionResizeMode to set_section_resize_mode. This wouldn't make any sense.

We could very well conform to mixedCase, such as renaming load_ui into loadUi. The translate function is already named correctly for both snake_case and mixedCase.

But when talking about the functions within Qt.py, tests.py, run_tests.py, written by us, I would propose we should keep it snake_case. This all means the following;

  • User-facing functions are mixedCase, example: QtWidgets, QtCompat.loadUi
  • Internal functions, variables, dicts etc are snake_case, example: _pyside2(), _bindings, _strict_members
  • All classes are always mixedCase (I don't think we have any today)

@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Feb 1, 2017

Also, we could rename load_ui into something which cannot easily be confused with loadUi(which exists in PyQt4.uic.loadUi). We could call it e.g. loadUiFile.

@bpabel
Copy link

bpabel commented Feb 1, 2017

I'm not sure you need to change the naming, since it's essentially replacing that function and the namespace will be different.

@mottosso
Copy link
Owner Author

mottosso commented Feb 2, 2017

Ok, sounds like we're in agreement on this convention.

from Qt import QtCompat
QtCompat.loadUi(...)
QtCompat.setSectionResizeMode(...)

And we'll let the namespace be the differentiator to whether it is an original or wrapped function.

Any additional thoughts, feel free to add.

mottosso added a commit to abstractfactory/Qt.py that referenced this issue Feb 9, 2017
@mottosso mottosso mentioned this issue Feb 9, 2017
mottosso added a commit to abstractfactory/Qt.py that referenced this issue Mar 23, 2017
mottosso added a commit that referenced this issue Mar 23, 2017
@fredrikaverpil
Copy link
Collaborator

fredrikaverpil commented Mar 28, 2017

I'm guessing we can close this, right?

@mottosso
Copy link
Owner Author

Indeed!

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

No branches or pull requests

3 participants