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

[processing] show warning when file-based layer could not be loaded #8967

Merged
merged 2 commits into from
Jan 29, 2019
Merged

[processing] show warning when file-based layer could not be loaded #8967

merged 2 commits into from
Jan 29, 2019

Conversation

volaya
Copy link
Contributor

@volaya volaya commented Jan 24, 2019

Description

Show warning when file-based layer could not be loaded and dependent params updated

For algorithms with multiple parameters depending on a vector layer parameter, the code that loads the layer in the background is called repeatedly, impacting performance. A small layer cache is implemented with these changes, so the dialog only tries to load the layer once.

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include fixes #11111 in the commit message next to the description
  • Commits which add new features are tagged with [FEATURE] in the commit message
  • Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and contain sufficient information in the commit message to be documented
  • I have read the QGIS Coding Standards and this PR complies with them
  • This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit

…nd dependent params updated

For algorithms with multiple parameters depending on a vector layer parameter, the code that loads the layer in the background is called repeatedly, impacting performance. A small layer cache is implemented with these changes, so the dialog only tries to load the layer once.
@@ -1615,14 +1620,26 @@ def postInitialize(self, wrappers):
break

def parentValueChanged(self, wrapper):
self.setLayer(wrapper.parameterValue())
value = wrapper.parameterValue()
if value in wrapper.fileBasedLayers:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this code for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the loaded layer is in the cache, it sets the widget layer with it instead of the parameter value. Otherwise, it just calls set layer with value itself.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not directly required by this PR though, is it? The QgsProcessingUtils.mapLayerFromString call used in setLayer already effectively uses a cache inside self.context, so we end up double-caching here, at a cost of more code complexity.

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 really required, but the performance improvement is noticeable. Otherwise, for an algorithm such as "Create point layer from table", with 4 dependent fields, mapLayerFromString is called 4 times, and the warning message is shown with a rather long delay.

I didnt know there was a cache in that method, but looks like it is not working in this case. Maybe mapLayerFromString is not caching when the layer cannot be read (wrong layer file, as in this case), and for this reason it tries 4 times to load the wrong layer (all of them unsuccessfully)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe mapLayerFromString is not caching when the layer cannot be read

Ah, you're right. All good here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants