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 expression functions to create CRS from string definition and to get CRS authid (refs #58313) #60485

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alexbruy
Copy link
Contributor

@alexbruy alexbruy commented Feb 6, 2025

Description

Processing batch mode allows to defined output name using expressions and all algorithm parameters are available as values in this context. However, CRS parameters when used in expression return nothing. This happens because when algorithm scope is populated we create variables using the values obtained from parametersForRow() method and these values are just uses as-is

for param in self.alg.parameterDefinitions():
if param.isDestination():
continue
col = self.parameter_to_column[param.name()]
wrapper = self.wrappers[row][col]
parameters[param.name()] = wrapper.parameterValue()

In case of CRS parameter, variable will be populated with the instance of QgsCoordinateReferenceSystem. Expression engine does not have support for variants containing QgsCoordinateReferenceSystem and evaluating such variable returns nothing as reported in #58313.

This PR adds two new expression functions:

  • make_crs(definition) to create a QgsCoordinateReferenceSystem from a definition
  • crs_authid(crs) to get a string with the CRS authid

Refs #58313.

@alexbruy alexbruy added Expressions Related to the QGIS expression engine or specific expression functions Processing Relating to QGIS Processing framework or individual Processing algorithms labels Feb 6, 2025
@alexbruy alexbruy requested a review from nyalldawson February 6, 2025 12:22
@github-actions github-actions bot added this to the 3.42.0 milestone Feb 6, 2025
@alexbruy
Copy link
Contributor Author

alexbruy commented Feb 6, 2025

@nyalldawson what do you think?

Another approach would be to evaluate parameter value for CRS parameter in the BatchPanel and use authid when settings corresponding variables.

Copy link

github-actions bot commented Feb 6, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 5e28726)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 5e28726)

src/core/expression/qgsexpression.cpp Outdated Show resolved Hide resolved
src/core/expression/qgsexpressionutils.h Show resolved Hide resolved
src/core/expression/qgsexpressionfunction.cpp Outdated Show resolved Hide resolved
@nyalldawson
Copy link
Collaborator

Can you also add the expression function json documentation for these?

@nyalldawson
Copy link
Collaborator

@nirvn also keen to hear your thoughts

@nirvn
Copy link
Contributor

nirvn commented Feb 10, 2025

My 2 cents:

  • I can see use cases for these functions, so that's all good :) but, I do think it'd require us to insure relevant geometry related functions are updated to handle CRS as parameters (e.g. transform() immediately comes to mind here)
  • For the expression naming, Could we go for something that resembles the pre-existing geom_{to,from}wkt naming convention a bit more. Maybe something like crs_{to,from}_description?

@alexbruy
Copy link
Contributor Author

alexbruy commented Feb 10, 2025

* I can see use cases for these functions, so that's all good :) but, I do think it'd require us to insure relevant geometry related functions are updated to handle CRS as parameters (e.g. transform() immediately comes to mind here)

Should we create another function that accepts crs objects or just modify existing one to accept either string authid or crs object? Is it ok to do this in a follow-up PR?

* For the expression naming, Could we go for something that resembles the pre-existing geom_{to,from}wkt naming convention a bit more. Maybe something like crs_{to,from}_description?

I'm ok with renaming make_crs to crs_from_description as description can be any supported by createFromString() string. But crs_to_description IMHO is too wide name for a function that returns just authid, maybe we can create another function which will return CRS description in different formats, e.g. wkt, proj, etc.

@nyalldawson
Copy link
Collaborator

I personally don't think _from_description / _to_description are precise enough. Eg the current "make_crs" function can create it from any form of text, eg a proj string or wkt string or authid (none of which are a "description"). So instead I'd propose crs_from_text for that one.

And I think the "to_text" variant needs to be more explicit, because again we could potentially have crs_to_wkt, crs_to_proj, crs_to_authid, crs_to_name, etc. So how about crs_to_authid for this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Expressions Related to the QGIS expression engine or specific expression functions Processing Relating to QGIS Processing framework or individual Processing algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants