Improve code style

[Jaime R.]

Hello, I have just started contributing to PyQtGraph. I was going to remove the import * statements, but I have seen tons of PEP-8 style violations. The only code style guidelines I have found so far are this ones:

Formatting Rules Suggestions

• PyQtGraph prefers PEP8 for most style issues, but this is not enforced rigorously as long as the code is clean and readable.
• Variable and Function/Methods that are intended to be part of the public API should be camelCase.
• "Private" methods/variables should have a leading underscore (_) before the name.

Why don't we embrace PEP-8 style? Tools like black makes it very easy to use. camelCase can be accepted because Qt uses it and otherwise it can be a mess, but things like ## comments are not good, right?

[Jaime R.]

For example, this is the first file I have improved:

https://github.com/Jaime02/pyqtgraph/commits/fix-formatting

Shall this be done everywhere? I could do it, It requires some work but I want to improve this things, I think it's worth.

[Ognyan Moore]

Hi Jaime,

I am one of the maintainers of pyqtgraph.  We've largely held off from any kind of massive automated formatting efforts as it would create merge conflicts with existing PRs, which we have been chipping away at for a number of years now (we used to have almost 170 open PRs, currently under 30 half of which are draft PRs).  The other reason is that pep8 doesn't quite line up with the Qt framework.  For example, the Qt framework uses camelCase where as pep8 dictates snake_case.  Due to our integration and dependence on the Qt framework, we adopt a lot of their naming convention.

When the PR count is effectively zero, we will be running black and pyupgrade on the entire codebase, and fixup any remaining flake8 errors; at which point we will also setup automated CI checks to check for those things (or perhaps use pre-commit.ci to enforce those standards).  We will need to do this carefully though, as running black on the entire codebase along would just entirely hose git-blame (there are ways to work around this).  On the branch you linked to, I would be inclined to not merge that presently, as it would likely result in some merge conflicts for other PRs that change GraphicsScene.py, and any kind of git-blame check would point back to the commit you made as it stands now.

That said, we have done some formatting tasks for the entire codebase, not long ago we ran isort over the entire codebase, as well as pycln.  Due to the limited scope of those runs, we were able to run those tasks w/o significant consequences to existing PRs.

Hope that answers your question and addresses your concern.

--
You received this message because you are subscribed to the Google Groups "pyqtgraph" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pyqtgraph+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/pyqtgraph/87cf2dd8-f4ba-4ca6-aa5a-e43b13d50b26n%40googlegroups.com.

[Jaime R.]

Hello, thanks for that detailed reply, I understand. At least shall I fix the import * imports? I'm looking for beginner-friendly tasks to do

[Ognyan Moore]

the import * lines we would likely want to preserve in __init__.py files; but outside of those, I would be good w/ removing them.

If you're interested in beginner-friendly issues to resolve, we have right now 13 "good first issue" labeled issues: https://github.com/pyqtgraph/pyqtgraph/labels/good%20first%20issue

(there are likely more than those 13 of those, it's just that I identified).

Ogi

To view this discussion on the web visit https://groups.google.com/d/msgid/pyqtgraph/02d6c246-d310-4694-b6ce-c699f76a6ffbn%40googlegroups.com.