Organize paramtypes by ntjess · Pull Request #1919 · pyqtgraph/pyqtgraph

ntjess · 2021-07-24T02:52:41Z

To prevent the extraordinary merge hassle of last parameter type PR, this organization:

Makes a new file for each parameter type
Allows parameter items to be registered without belonging to a parameter
Splits WidgetParameterItem implementations into actually separate items like BoolParameterItem, ColorParameterItem, etc.
A script to auto-generate parameter types documentation

For reviewers: The content of each Parameter/Item is unchanged -- the only modifications are to the imports

ntjess · 2021-07-24T03:01:51Z

@j9ac9k sorry to increase the PR count so soon 😆

ntjess · 2021-07-24T03:10:50Z

@ixjlyons Any idea why the docs failed? It says there is a problem but I'm not sure how to find out what it was...

j9ac9k · 2021-07-24T03:21:07Z

From the docs build:

/home/docs/checkouts/readthedocs.org/user_builds/pyqtgraph/checkouts/1919/doc/source/../../pyqtgraph/parametertree/parameterTypes/basetypes.py:docstring of pyqtgraph.parametertree.parameterTypes.basetypes.SimpleParameter:5: WARNING: Unexpected indentation.
/home/docs/checkouts/readthedocs.org/user_builds/pyqtgraph/checkouts/1919/doc/source/../../pyqtgraph/parametertree/parameterTypes/numeric.py:docstring of pyqtgraph.parametertree.parameterTypes.numeric.NumericParameterItem:5: WARNING: Unexpected indentation.
/home/docs/checkouts/readthedocs.org/user_builds/pyqtgraph/checkouts/1919/doc/source/../../pyqtgraph/parametertree/parameterTypes/numeric.py:docstring of pyqtgraph.parametertree.parameterTypes.numeric.NumericParameterItem:6: WARNING: Block quote ends without a blank line; unexpected unindent.

I believe we have our sphinx config set to error on warning.

j9ac9k · 2021-07-24T03:22:02Z

@j9ac9k sorry to increase the PR count so soon 😆

You're not going to hear me complain about a PR that will make the library easier to maintain.

ntjess · 2021-07-24T03:24:35Z

@NilsNemitz Right, I didn't realize until Ogi's comment that warnings equate to errors with the current build specifications

ntjess · 2021-07-24T03:27:55Z

I'm also a little confused... Where is the unexpected indent? It looks similar to other documentation. It's not tabs either, just making sure

j9ac9k · 2021-07-24T03:38:06Z

I would recommend migrating to numpy-docstyle ...we're slowly moving over in that direction, it's a bit easier to read, preserves horizontal space. We haven't been enforcing it, given much of the library isn't of that format. TargetItem.py has some examples of how to format there that may be useful. I think PyCharm (or really any popular IDE/editor) has extensions to auto-populate/format a stub that you can just fill in.

But to directly answer your question, I'm not sure what sphinx is talking about (my doc-formatting game is weak). I can recreate the doc-build environment and help diagnose in a bit.

NilsNemitz · 2021-07-24T06:09:59Z

Dear @ntjess , I think it might mostly be complaining about the missing blank line.
And it might be expecting that blank line before the
========= =================== section.

pyqtgraph/parametertree/parameterTypes/basetypes.py

ixjlyons · 2021-07-31T16:10:43Z

doc/source/parametertree/parametertypes.rst

 Parameters
 ----------

-.. autoclass:: SimpleParameter


I have somewhat of a preference for keeping these as just the ClassName here. The difference is the signature rendered in the docs:

class pyqtgraph.parametertree.parameterTypes.SliderParameter(**opts)

vs.

class pyqtgraph.parametertree.parameterTypes.slider.SliderParameter(**opts)

Since they will almost always be imported via from pyqtgraph.parametertree import parameterTypes, the old version may be a little less confusing.

Currently the items are ordered by appearance in PARAM_TYPES. Do you think they should be ordered alphabetically instead while I'm making changes? Not sure which is preferable.

I don't have a strong preference. I guess I'd lean toward alphabetical so it's a little easier to find something when scrolling, but if I know what I'm looking for I'd probably ctrl+f or use the search box anyway.

Scratch that -- I thought I took out set logic but that means order changes almost every time the script is run... I think I will sort so the order is deterministic and makes clear where new parameters should be added

pyqtgraph/parametertree/Parameter.py

ixjlyons · 2021-07-31T16:34:39Z

tools/rebuildPtreeRst.py

@@ -0,0 +1,42 @@
+import os.path


Script seems fine to me, just highlights that we really should try to migrate toward using sphinx-autogen/autosummary. My issue is it usually seems to require some templating/theming which isn't really a passion of mine.

Yeah, this is more of a stand-in but certainly not the best long-term solution

tools/rebuildPtreeRst.py

- `registerParameterItemType`: * added to docs and parametertree.__init__ * Remove unsed PARAM_TYPES global * Hyperlink to `registerParameterType` - parameter tree rst: * Alphabetize entries * Rebuild RST without fully qualified class name * Add note at file header that it is auto generated

j9ac9k · 2021-08-01T03:57:24Z

pyqtgraph/parametertree/parameterTypes/basetypes.py

+
+    def setAddList(self, vals):
+        """Change the list of options available for the user to add to the group."""
+        self.setOpts(addList=vals)


nit-pick; looks like this file needs a newline at the end.

EDIT: a number of files look like they need new-lines after the last line of code.

I didn't realize this was a requirement, apologies. I'll configure my settings accordingly

I ran a script which found a few other repo files without newlines. Should a quick PR be added for them?

examples/exampleLoaderTemplate_pyside2.py examples/exampleLoaderTemplate_pyside6.py examples/initExample.py examples/optics/__init__.py examples/verlet_chain/__init__.py pyqtgraph/canvas/__init__.py pyqtgraph/console/__init__.py pyqtgraph/dockarea/__init__.py pyqtgraph/flowchart/__init__.py pyqtgraph/imageview/__init__.py pyqtgraph/ordereddict.py pyqtgraph/util/get_resolution.py pyqtgraph/util/pil_fix.py pyqtgraph/widgets/FeedbackButton.py pyqtgraph/widgets/VerticalLabel.py tests/test_Point.py

j9ac9k · 2021-08-01T03:59:55Z

My only issue here is lack of \n at the end of the files; also the static code checker picked up some unused imports that should probably be removed.

ntjess · 2021-08-01T18:06:45Z

@j9ac9k Wow, CodeQL is actually helpful when it doesn't trigger all 300 warnings across the project for an unrelated PR 😆

j9ac9k · 2021-08-02T03:30:26Z

@ntjess if you think this guy is ready for merging, probably should remove the draft status 👍🏻

j9ac9k · 2021-08-02T05:42:42Z

This PR is now showing merge conflicts (likely having to do with #1939 ) ...sorry for the trouble @ntjess ! Once that's resolved we can merge.

…organize-paramtypes

Note: intentionally leaves out instance in parametertypes.py to avoid merge conflict with pyqtgraph#1919

ntjess · 2021-08-02T15:36:17Z

@ksunden If you used a script to find unneeded castings before, it might be good to run the same script on this branch to make sure I didn't miss anything

Note: intentionally avoids files moved / already adjusted from pyqtgraph#1919

prior to PR pyqtgraph#1919, there was no ptree.types.ColorMapParameter, and type "colormap" was handled directly by ptree.types.SimpleParameter. PR pyqtgraph#1919 created ColorMapParameter as a sub-class of SimpleParameter, and moved colormap specific functionality into ColorMapParameter. Hence RangeColorMapItem now needs to derive from ColorMapParameter.

ntjess added 7 commits July 23, 2021 20:08

Registered parameter types go in their own files

c8869bf

Moves [int, float] item definitions outside WidgetParameterItem

be06c3d

Moves [int, float] parameter definitions outside WidgetParameterItem

9db164f

Allow registering ParameterItems for easy parameter defs

fb8cb32

Finalizes moving simple parameters to their own files

662ae55

removes accidentally committed file

41cf702

Provides class qualnames in rst

2e64304

NilsNemitz reviewed Jul 24, 2021

View reviewed changes

pyqtgraph/parametertree/parameterTypes/basetypes.py Show resolved Hide resolved

ntjess force-pushed the organize-paramtypes branch from e1a6189 to bfbdd8c Compare July 24, 2021 14:36

Address docstring build issues

c813e0f

ntjess force-pushed the organize-paramtypes branch from bfbdd8c to c813e0f Compare July 24, 2021 14:45

ixjlyons reviewed Jul 31, 2021

View reviewed changes

ntjess force-pushed the organize-paramtypes branch from de378df to 174d4cf Compare July 31, 2021 22:03

ixjlyons approved these changes Jul 31, 2021

View reviewed changes

j9ac9k reviewed Aug 1, 2021

View reviewed changes

ntjess added 3 commits August 1, 2021 14:06

Remove spurious space during rst doc creation

3ef0b87

Ensure created/modified files end with newline

6d0907f

Address CodeQL warnings

a3c514a

ntjess marked this pull request as ready for review August 2, 2021 05:03

ntjess mentioned this pull request Aug 2, 2021

Remove all usage of python2_3.py #1939

Merged

ntjess added 2 commits August 2, 2021 11:03

Merge branch 'master' of https://github.com/pyqtgraph/pyqtgraph into …

25e2cb0

…organize-paramtypes

toPlainText also returns str

3f2c211

ntjess added a commit to ntjess/pyqtgraph that referenced this pull request Aug 2, 2021

toPlainText also returns str

a781550

Note: intentionally leaves out instance in parametertypes.py to avoid merge conflict with pyqtgraph#1919

ntjess added a commit to ntjess/pyqtgraph that referenced this pull request Aug 2, 2021

toPlainText also returns str

84b491f

Note: intentionally leaves out instance in parametertypes.py to avoid merge conflict with pyqtgraph#1919

ntjess mentioned this pull request Aug 2, 2021

Remove unnecessary casting of toPlainText to str #1941

Merged

ntjess added a commit to ntjess/pyqtgraph that referenced this pull request Aug 2, 2021

Adds EOF newline to files missing it.

ef99d3f

Note: intentionally avoids files moved / already adjusted from pyqtgraph#1919

ntjess mentioned this pull request Aug 2, 2021

Adds EOF newline to files missing it #1942

Merged

QTreeWidgetItem.text returns str

6852bfd

j9ac9k merged commit 4bf1866 into pyqtgraph:master Aug 2, 2021

ntjess deleted the organize-paramtypes branch August 5, 2021 13:24

pijyoi mentioned this pull request Nov 11, 2021

examples/ScatterPlotWidget.py Color Map throws exception #2070

Closed

pijyoi mentioned this pull request Dec 2, 2021

RangeColorMapItem derives from ptree.types.ColorMapParameter #2130

Merged

Uh oh!

Conversation

ntjess commented Jul 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntjess commented Jul 24, 2021

Uh oh!

ntjess commented Jul 24, 2021

Uh oh!

j9ac9k commented Jul 24, 2021

Uh oh!

j9ac9k commented Jul 24, 2021

Uh oh!

ntjess commented Jul 24, 2021

Uh oh!

ntjess commented Jul 24, 2021

Uh oh!

j9ac9k commented Jul 24, 2021

Uh oh!

NilsNemitz commented Jul 24, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

j9ac9k Aug 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

j9ac9k commented Aug 1, 2021

Uh oh!

ntjess commented Aug 1, 2021

Uh oh!

j9ac9k commented Aug 2, 2021

Uh oh!

j9ac9k commented Aug 2, 2021

Uh oh!

ntjess commented Aug 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ntjess commented Jul 24, 2021 •

edited

Loading

j9ac9k Aug 1, 2021 •

edited

Loading