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 #55894

Merged
merged 14 commits into from
Feb 8, 2024
Merged

Add g.version algorithm #55894

merged 14 commits into from
Feb 8, 2024

Conversation

AlisterH
Copy link
Contributor

@AlisterH AlisterH commented Jan 19, 2024

Description

  • Add g.version algorithm, allowing the user to check the details of the GRASS version that the GRASS processing provider is using (useful for debugging).
  • Update the grass processing algorithms guide.
  • Bugfix: prevent the algorithm dialog from reporting html output has been created if the user chose not to create it.
  • Update g.extension.list to use QgsProcessingParameterEnum
  • Save output of g.version and g.extension.list algorithms to an output string when creating html output.
    This allows code to get the actual version number:
  >>> processing.run("grass7:g.version",{'html': 'grass_version_info.html', '-r': 0, '-e':0})['GRASS_VERSIONINFO'].split()[1]
  '8.3.1'

It also allows code to install a grass addon if it isn't already installed:

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

History

Notes

  • I would like to print out the GRASS_VERSIONINFO and GRASS_ADDONS outputs when running in the algorithm dialog, so that users know they exist. The original pull request achieved this (although they weren't listed by .algorithmHelp), but I'm not sure how I should achieve this now - I think it may be related to this part in src/core/processing/qgsprocessingfeedback.cpp:
  const QList< const QgsProcessingOutputDefinition * > outputs = algorithm->outputDefinitions();
  for ( const QgsProcessingOutputDefinition *output : outputs )
  • Ideally these outputs would be created even when the user elects not to create html output, so that when running in code you don't need to create the html files. But that was too difficult for me to implement, and unless it is easier than it seemed, it would be hardly worth someone else's time to work on it.

@github-actions github-actions bot added this to the 3.36.0 milestone Jan 19, 2024
Copy link

github-actions bot commented Jan 19, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit a2a7d2e)

@AlisterH
Copy link
Contributor Author

Notes

  • I would like to print out the GRASS_VERSIONINFO and GRASS_ADDONS outputs when running in the algorithm dialog, so that users know they exist.

Having said that, I don't think there's any reason this couldn't be rectified at a later date. But if there's a preference to use one standard name such as MESSAGES it would be better to make that decision first.

The original pull request achieved this, but I'm not sure how I should achieve this now - I think it is due to this part in src/core/processing/qgsprocessingfeedback.cpp:

  const QList< const QgsProcessingOutputDefinition * > outputs = algorithm->outputDefinitions();
  for ( const QgsProcessingOutputDefinition *output : outputs )
  • Ideally these outputs would be created even when the user elects not to create html output, so that when running in code you don't need to create the html files. But that was too difficult for me to implement, and unless it is easier than it seemed, it would be hardly worth someone else's time to work on it.

Actually, my first thought was that it would be good to always (for all GRASS algorithms) save the messages GRASS prints to an output called 'MESSAGES' or something, but I thought there may be a reason for an algorithm to create more than one output like this, and in any case I decided I wasn't proficient enough to mess with the code that prints the GRASS outputs to the dialog when there is no html output.

@nyalldawson nyalldawson added the Freeze Exempt Feature Freeze exemption granted label Feb 1, 2024
@@ -340,6 +341,7 @@ def on_complete(ok, results):

def finish(self, successful, result, context, feedback, in_place=False):
keepOpen = not successful or ProcessingConfig.getSetting(ProcessingConfig.KEEP_DIALOG_OPEN)
HtmlOutputs = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you open a separate PR for this fix? It's unrelated to the grass ones

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you open a separate PR for this fix? It's unrelated to the grass ones

#56115

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! Can you update this PR and remove that commit now?

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've reverted, squashed and force-pushed. Hopefully that's the proper way to do this!

@nyalldawson
Copy link
Collaborator

@AlisterH can you update this? Some of these files moved around in #56196

@AlisterH
Copy link
Contributor Author

AlisterH commented Feb 7, 2024

Done, but if I need to amend those commit messages that refer to grass7.txt I'll need to come back tomorrow to do it.

python/plugins/grassprovider/grass.txt Outdated Show resolved Hide resolved
@nyalldawson nyalldawson merged commit a5cfd70 into qgis:master Feb 8, 2024
29 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants