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 g.version algorithm #53635

Closed
wants to merge 12 commits into from
Closed

Add g.version algorithm #53635

wants to merge 12 commits into from

Conversation

AlisterH
Copy link
Contributor

Description

This allows the user to check the details of the GRASS version that the GRASS processing provider is using. Useful for debugging.

@github-actions github-actions bot added this to the 3.34.0 milestone Jun 28, 2023
@github-actions github-actions bot added the GRASS label Jun 28, 2023
@nyalldawson
Copy link
Collaborator

Ideally all these values would become outputs from this algorithm (so eg a model can adapt dynamically). Would you be willing to implement that?

@AlisterH
Copy link
Contributor Author

AlisterH commented Jun 29, 2023

I'm not sure I understand what you're suggesting.
Whatever flags you use, g.version just produces one output. So if you want separate outputs, one for copyright message, one for citation options, etc, you would run it multiple times with different flags, which sounds like a good job for a processing model. But I'm not sure that it would be useful for adaptable models.

Surely for adaptable models all you need is to:

  • parse the grass version number (which you get by running this with all the options unchecked), and/or
  • parse the output of g.extension -a, which you get using g.extension.list algorithm that we committed not long ago.

My next thought was to look at a standard tool to check if a particular addon is available, and if not try to install it. I think that would be most useful.

@AlisterH
Copy link
Contributor Author

AlisterH commented Jun 29, 2023

Ideally perhaps what we should do for both g.version and g.extension.list is detect whether it is running as part of a model and if so store the results in an output variable rather than an output file.
At some point I might try to see if it is possible to do that using the ext file, but it may be over my head.

@AlisterH
Copy link
Contributor Author

I did spend quite some time looking at always saving the algorithm stdout to an output variable - I can just follow the existing code we're interacting with, but it's too complicated for me figure out the best way to do this.

@nyalldawson
Copy link
Collaborator

What I mean is that a html output is rather a dead end -- it's useful for a manual check of something, but restricts any use in automation (models or scripts). It would be better if we included the parsing in the algorithm so that the algorithm includes a string (or number) output value which is then super easy for use by these tools.

@AlisterH
Copy link
Contributor Author

AlisterH commented Jul 3, 2023

I've now committed changes to store the output in an output variable as well as to html.
I've also changed the way outputs are printed, as I don't think pformat was acceptable for these outputs.
This aspect of the implementation may be contentious, but it works.

@AlisterH
Copy link
Contributor Author

AlisterH commented Jul 3, 2023

With this it is easy enough to get the actual version number:

processing.run("grass7:g.version",{'html': 'grass_version_info.html', '-r': 0, '-e':0})['MESSAGES'].split()[1]

'7.8.7'

If we can get #53246 merged, a QGIS plugin could provide an algorithm description for e.g. r.stream.order, and simply do this to ensure the GRASS addon is installed:

alglist=processing.run("grass7:g.extension.list",{'-a': 'TRUE', 'html': 'addons_list.html'})['MESSAGES']
if 'r.stream.order' not in alglist.splitlines():
 processing.run("grass7:g.extension.manage",{'extension': 'r.stream.order', 'operation': 0})
{}

Since we don't provide any output to show whether an addon install or uninstall was successful, it might be wise for anyone doing this to then check again if it is actually installed...

@github-actions
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added stale Uh oh! Seems this work is abandoned, and the PR is about to close. and removed stale Uh oh! Seems this work is abandoned, and the PR is about to close. labels Jul 24, 2023
@AlisterH
Copy link
Contributor Author

Bumping so the bot doesn't kill us.

I've now committed changes to store the output in an output variable as well as to html. I've also changed the way outputs are printed, as I don't think pformat was acceptable for these outputs. This aspect of the implementation may be contentious, but it works.

Also, I should note that there is something really inefficient about the code responsible for printing output messages to the algorithm dialog when we aren't creating an html output. Algorithms which produce a lot of output messages seem to speed up dramatically if generating an html output instead of printing to the algorithm dialog.

@@ -0,0 +1,11 @@
g.version
g.version - Display GRASS GIS version info. <p>Prints only version if run with no options checked.
Copy link
Member

Choose a reason for hiding this comment

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

Same.
QgsProcessingParameterEnum with multiple choices

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would need some more guidance on how to enable "multiple choices", but in any case, aren't checkboxes more user-friendly?

@github-actions
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 22, 2023
@AlisterH
Copy link
Contributor Author

.

@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 27, 2023
@github-actions
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 11, 2023
@nyalldawson nyalldawson added the Freeze Exempt Feature Freeze exemption granted label Sep 15, 2023
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 16, 2023
@github-actions
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 30, 2023
@github-actions
Copy link

github-actions bot commented Oct 8, 2023

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

@github-actions github-actions bot closed this Oct 8, 2023
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Freeze Exempt Feature Freeze exemption granted GRASS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants