Add an option to delete unused categories in the categorized symbol renderer widget #60641
Conversation
|
I note that #60546 also modifies qgscategorizedsymbolrendererwidget.cpp. If the other PR is merged first, I am happy to resolve any conflicts and update this PR. |
🪟 Windows buildsDownload Windows builds of this PR for testing. 🪟 Windows Qt6 buildsDownload Windows Qt6 builds of this PR for testing. |
benwirf
force-pushed
the
delete_unused_renderer_categories
branch
from
245a43b to
3c94b36
Compare
|
@troopa81, thanks for the review- I have rebased & updated with your suggestions. I don't understand the changes to the .ui file though. All I do is add a push button to the horizontal layout below the tree view widget. Also not sure why the layout/documentation checks are failing. Any ideas? |
|
OK, I think I've sorted out the issues with the .ui file. Files changed is only showing the inserted tags for the new push button. I also made sure the pre-commit hooks were installed as per: https://github.com/qgis/QGIS/blob/master/CONTRIBUTING.md before my last commit, but Code Layout/ documentation checks is still failing. I'm not sure why. |
You have undocumented method. I think it's deleteUnusedCategories() even if it's not exposed to python API. You can reproduce it on your side with the following command in the build directory |
| * | ||
| * Called by addCategories() and deleteUnusedCategories() | ||
| */ | ||
| QList<QVariant> layerUniqueValues( const QString attrName ); |
There was a problem hiding this comment.
| QList<QVariant> layerUniqueValues( const QString attrName ); | |
| QList<QVariant> layerUniqueValues( const QString& attrName ); |
…to delete unused categories from the model/view. I.e. any categories which are not matched in the attribute field or expression used to categorize the layer.
benwirf
force-pushed
the
delete_unused_renderer_categories
branch
from
bb09a74 to
dc70921
Compare
Doh! Thanks @troopa81. |
| if ( idx == -1 ) | ||
| { | ||
| // Lets assume it's an expression | ||
| QgsExpression *expression = new QgsExpression( attrName ); |
There was a problem hiding this comment.
You have à memory leak here, please use std::unique_ptr here or remove the ptr .
There was a problem hiding this comment.
Ok, I have no problem with these suggestions and I'm happy to make these changes, but please note that this is part of the block of code which I reused from the existing addCategories() method. I.e. this code is in the current master branch:
Again, I'm happy to change it as part of this PR- is there anything else that you think should be modified here?
There was a problem hiding this comment.
Ideally, I would have spotted during my first review , but thank you for making the modification, one less memory leak.
Lgtm
There was a problem hiding this comment.
Absolutely! Thanks again for your help @troopa81.
There was a problem hiding this comment.
Hi @troopa81, even though this PR is closed I just wanted to leave this here as a follow up. Firstly, I appreciate the time you gave me here- I should have picked these things up myself! As you can probably tell I'm fairly inexperienced in C++, but your reviews here prompted me to think/learn a bit more about memory management, heap/stack allocation etc. and to be a bit more analytical/critical when modifying existing QGIS code which will help me to submit better PRs in the future. So I just wanted to say thank you.
There was a problem hiding this comment.
@benwirf You're welcome! I'm always happy to see new contributors. Keep up the good work
Description
This PR adds an option via a button in the categorized symbol renderer widget to delete unused categories from the model/view. I.e. any categories which are not matched in the attribute field or expression used to categorize the layer.
Example use case:
I have a layer of land systems covering the entire Northern Territory and a .qml style file which categorizes this layer into some 22 classes based on an attribute "CLASS".
Now I clip this NT-wide layer to a much smaller area. The subset only contains 7 of these classes, however I still want to load the style from the same .qml style file.
With the added option, one can quickly delete all the categories which are not used by the smaller subset layer:
In the added
void QgsCategorizedSymbolRendererWidget::deleteUnusedCategories()method, I reused a block of code from the existingaddCategories()method to build aQList<QVariant>of unique values in the layer based on the current classification attribute or expression. To avoid duplication & to try and keep things tidy, I moved this block to a new protected method:QList<QVariant> QgsCategorizedSymbolRendererWidget::layerUniqueValues( QString attrName )which returns the list of unique values and is then called in bothaddCategories()anddeleteUnusedCategories().