Some bindings issues:

1. PySide{2, 6} does not have QVariant
2. PyQt6 requires long spelling for enums (see Convert Qt Enums to Qt6 Namespace #1818)

@pijyoi wow, turns out combo boxes implicitly convert enums to ints, fun...

It should be fixed now, thanks.

Ok to merge? Suggestions? Comments? I want to do some housekeeping on parameter types but also want to wait until this is resolved.

I haven't looked over the code in detail yet, but a few high-level notes from playing around with the example:

• The pen parameter should probably have the parameter name in col 0 and the button in col 1, and maybe a default button as well - it seems like it could be implemented as a WidgetParameterItem? I suppose the intent may have been that it's clear enough you're looking at a button with a "pen symbol" but in a GUI you might want multiple pen controls for different things.
• The pen doesn't show display with its default value at first - i.e. if you open the pen dialog and cancel, it will update to red (the default).
• The file picker would probably work best as the traditional line edit + "browse" button combo. Horizontal space in a parameter tree can get cramped, but having the file path in the push button isn't very effective.
• We'll need a doc string for each new item that I suppose should just highlight any non-standard options, then add the new parameters/items to doc/source/parametertree/parameterTypes.rst. At a glance, the file picker looks like the only one with new options.

@ixjlyons Thanks for the feedback. Do you recommend the current value of the file picker not show up anywhere visually? I'm ok with it and it's an easy change, I just wanted to be sure. Some alternatives are to just show the filename instead of full path or use ellipses (change elide mode on the label)

• Are you OK with the file API -- existing, asFolder, fileFilter, selectMultiple, startDir? I.e., should any names be changed for clarity?
  • Currently fileFilter requires the same format as Qt (e.g. text files (*.txt);; ...), should a different format be allowed / provisioned for easier use?
• Is DontUseNativeDialog common enough to expose at the parameter level? I know for me it caused some valuable differences in earlier Qt versions (<5.12), and people who want to add widgets to the filepicker cannot use the native choice.

@ixjlyons Thanks for the feedback. Do you recommend the current value of the file picker not show up anywhere visually? I'm ok with it and it's an easy change, I just wanted to be sure. Some alternatives are to just show the filename instead of full path or use ellipses (change elide mode on the label)

• Are you OK with the file API -- existing, asFolder, fileFilter, selectMultiple, startDir? I.e., should any names be changed for clarity?

This is the kind of API nitpick I can get behind!

• asFolder I would advice using "Directory" instead of "Folder".

Is there a reason we shouldn't match up the existing Qt API here and just use directory (instead of startDir), filter (instead of fileFilter) and so on? There is likely a good idea why not, it's not the sort of thing I know much about unfortunately.

• Currently fileFilter requires the same format as Qt (e.g. text files (*.txt);; ...), should a different format be allowed / provisioned for easier use?

Unless there is demand for another format, I would stick w/ that for a few reasons.

1. it should be familiar to people w/ Qt experience already
2. If someone learns about this format from pyqtgraph, they can apply it elsewhere in the Qt framework
3. Minimizes our support footprint

If there is a feature request to add another syntax, we can evaluate that on its own merit, but even if we come up with a potentially simpler format; there is likely no guarantee it would be simpler for enough use-cases to make it worthwhile.

• Is DontUseNativeDialog common enough to expose at the parameter level? I know for me it caused some valuable differences in earlier Qt versions (<5.12), and people who want to add widgets to the filepicker cannot use the native choice.

DontUseNativeDialog should likely go away, I think it was introduced in this library as a work-around for some issue on macOS eons ago.

@j9ac9k

Is there a reason we shouldn't match up the existing Qt API here and just use directory (instead of startDir),

directory may refer to either starting directory or whether a directory should be selected, so I tried to disambiguate the two.

filter (instead of fileFilter)

I tend not to use builtin functions as variables, which is why I stayed away from filter (but it makes sense here to match the Qt API).

and so on?

The other accepted arguments are boolean versions of enum flags which are easier to store in the Parameter state and slightly simpler (my initial reasoning). I could also accept fileMode/options/etc. argument with stringified versions of desired flags, i.e.

Parameter.create(type='file', fileMode='ExistingFiles', options=['ShowDirsOnly', 'DontResolveSymlinks'], viewMode='Detail')

Additionally, I could wait until #1120 is merged and use true bitmasks for these. (My stream of consciousness for including this was to have provisions for a bitmask during saveState, but #1120 is irrelevant because these flags wouldn't be edited within the file parameter. Hence, single string or list-of-strings is probably the way to go if we proceed with this)

• I could also override saveState to convert enums into string values before output. This means arguments like options can directly accept Option.DontResolveSymlinks|Option.ReadOnly. In general, though, I like the principle 'explicit is better than implicit' where the saved state is the same one input by the user when possible

FWIW, my initial reason for the boolean interface was simplicity and it might be intimidating to expose every aspect of a file dialog even if ~80% of it isn't used in common practice.

@j9ac9k

Is there a reason we shouldn't match up the existing Qt API here and just use directory (instead of startDir),

directory may refer to either starting directory or whether a directory should be selected, so I tried to disambiguate the two.

Ahh good point!

filter (instead of fileFilter)

I tend not to use builtin functions as variables, which is why I stayed away from filter (but it makes sense here to match the Qt API).

Oh, missed the part this was a variable name, yeah built-ins as variable names is a no-no, could do filter_ (the same way qt bindings have navigated around exec), but I'm not strongly opinionated on this and am good w/ what you think is best.

The other accepted arguments are boolean versions of enum flags which are easier to store in the Parameter state and slightly simpler (my initial reasoning). I could also accept fileMode/options/etc. argument with stringified versions of desired flags, i.e.

Parameter.create(type='file', fileMode='ExistingFiles', options=['ShowDirsOnly', 'DontResolveSymlinks'], viewMode='Detail')

Either option (list of bools, list of stringified enums) sounds good to me; probably should support default behavior, and put a link in the documentation to the Qt docs for more info.

• I could also override saveState to convert enums into string values before output. This means arguments like options can directly accept Option.DontResolveSymlinks|Option.ReadOnly. In general, though, I like the principle 'explicit is better than implicit' where the saved state is the same one input by the user when possible

FWIW, my initial reason for the boolean interface was simplicity and it might be intimidating to expose every aspect of a file dialog even if ~80% of it isn't used in common practice.

You've thought about this more than I have at this point, I'm good w/ it either way 👍🏻

@ixjlyons

The file picker would probably work best as the traditional line edit + "browse" button combo. Horizontal space in a parameter tree can get cramped, but having the file path in the push button isn't very effective.

Fair point, horizontal space is a premium in parameter trees. How about this? If you like it, I'll push the fix.

That seems reasonable, definitely an improvement.

It still seems like it'd be useful to have the path in a line edit so it can be manually changed or copied/pasted. Maybe a button with just ... to save space or maybe it could use asSubItem to span both columns?

I'm totally fine saving that as an enhancement idea for later.

Take 2

I also took the liberty of making a slightly more interactive pen parameter 🙂

I'm realizing right after pushing that a line edit will not interact nicely with ExistingFiles, unless I use ast.literal_eval and go down that rabbit hole... Perhaps make it readonly if multiple selection is enabled?

There's also no input sanitation currently, i.e. if you edit directly you can violate all constraints set on the parameter. I don't quite know what the ideal solution is other than to leave it for now. Maybe make it readonly by default, and the adventurous user can enable it if they so choose?

With that, I think I've addressed all comments from the previous round. @ixjlyons and @j9ac9k thanks for your prior input, are you up for another go?

File picker is readonly by default and no sanitation or completer is provided (I'm considering it out of scope for now, someone can submit a PR if they wish). Those wanting to copy-paste can specify readonly=False in the opts at their own risk.

With Scipy sprints and the new release coming up I know this will get delayed for a bit, any recommendations on when I can bump again? I'm looking to do a parameterTypes restructuring PR to avoid every new type modifying the same file. However, it would be easiest to wait until after this is addressed.

Thanks :)

Hi @ntjess I think the sprints, at least on Sunday, would be a great time to review this PR actually. @ixjlyons had an interest in revisiting the PR backlog, this issue, along w/ Nils's color palette one are some big PRs that would benefit from getting more eyes on IMO. Given many of us will be there, I think we can get some real-time feedback and hopefully merge 🤞🏻

Rebased onto master, merge conflicts should be gone but the CI failure is still occurring. Is that linked to any of my changes? I can't reproduce locally on my windows machine so it's difficult for me to analyze

with what appears to be a segfault, causes like this can be really tough to isolate; even worse when it only happens in CI but not locally... best you can hope for is it fails in CI consistently 😆

what a rollercoaster this one was....

A lot of thanks for this PR.

Thank you @feketeimre for the original submission
Thank you @ntjess for updating/modernizing + bearing through the CI issue
Thank you to the reviewers of this PR from the SciPy sprints.

Squashing and merging this one!