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

Add support for point cloud expressions to Processing expression parameter #52730

Merged
merged 9 commits into from
May 2, 2023
Merged

Add support for point cloud expressions to Processing expression parameter #52730

merged 9 commits into from
May 2, 2023

Conversation

alexbruy
Copy link
Contributor

Description

With addition of filtering capabilities to point cloud algorithms, we need a userfriedly way to build point cloud expressions similarly to the QGIS native expressions.

To make it possible this pull-request adds a Type enum to QgsProcessingParameterExpression parameter with two options Qgis and PointCloud. If parameter has a PointCloud type, it will use new widget wrapper with GUI point cloud expression builder.
2023-04-17-091207_1366x768_scrot
To maintain backward compatibility, by default expression parameter uses Qgis expression type.

The QgsPointCloudExpression class was extented with the new method to convert QGIS point cloud expressions to PDAL expressions.

All PDAL algorithms using expression filter were updated to use expression parameter instead of strings.

@alexbruy alexbruy added Feature Processing Relating to QGIS Processing framework or individual Processing algorithms Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. labels Apr 17, 2023
@github-actions
Copy link

@alexbruy
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@github-actions github-actions bot added this to the 3.32.0 milestone Apr 17, 2023
*
* \since QGIS 3.32
*/
QString asPdalExpression() const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs some bool& ok SIP_OUT, QString& error SIP_OUT arguments so that callers can get feedback if an expression can't be converted to PDAL format

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about it. Conversion is done similar to how the dump() method works, with occasional substring replacement (e.g. "and" is replaced with "&&").

src/core/processing/qgsprocessingparameters.cpp Outdated Show resolved Hide resolved
src/gui/processing/qgsprocessingwidgetwrapperimpl.cpp Outdated Show resolved Hide resolved
src/gui/processing/qgsprocessingwidgetwrapperimpl.cpp Outdated Show resolved Hide resolved
src/gui/processing/qgsprocessingwidgetwrapperimpl.cpp Outdated Show resolved Hide resolved
void QgsProcessingPointCloudExpressionLineEdit::editExpression()
{
const QString currentExpression = expression();
QgsProcessingPointCloudExpressionDialog dlg( mLayer );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be SUPER nice to make this open as a QgsPanelWidget. Then we could avoid an unwanted modal dialog blocking things.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind if I do it in a follow up pull-request?

@github-actions
Copy link

github-actions bot commented May 2, 2023

@alexbruy
A documentation ticket has been opened at qgis/QGIS-Documentation#8233
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@alexbruy alexbruy deleted the processing-pointcloud-expressions branch May 2, 2023 12:11
@zacharlie zacharlie added the Changelog Items that are queued to appear in the visual changelog - remove after harvesting label Jun 17, 2023
@qgis-bot
Copy link
Collaborator

@alexbruy

This pull request has been tagged for the changelog.

  • The description will be harvested so please provide a "nearly-ready" text for the final changelog
  • If possible, add a nice illustration of the feature. Only the first one in the description will be harvested (GIF accepted as well)
  • If you can, it's better to give credits to your sponsor, see below for different formats.

You can edit the description.

Format available for credits
  • Funded by NAME
  • Funded by URL
  • Funded by NAME URL
  • Sponsored by NAME
  • Sponsored by URL
  • Sponsored by NAME URL

Thank you!

@zacharlie zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels Jun 17, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ChangelogHarvested This PR description has been harvested in the Changelog already. Feature Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. Processing Relating to QGIS Processing framework or individual Processing algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants