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

Merge by form factor #24

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Merge by form factor #24

wants to merge 4 commits into from

Conversation

domi4484
Copy link
Member

@domi4484 domi4484 commented Feb 25, 2025

Add algorithm to merge elongated geometries under 10m² by form factor

Its disabled by default so Sergio can try it out and decide if activating or not

image

@domi4484 domi4484 marked this pull request as draft February 25, 2025 20:20
@domi4484 domi4484 force-pushed the merge-by-form-factor branch 2 times, most recently from 64a53d5 to abe6204 Compare March 12, 2025 09:03
@domi4484 domi4484 force-pushed the merge-by-form-factor branch from abe6204 to 3182c33 Compare April 2, 2025 05:46
@domi4484 domi4484 marked this pull request as ready for review April 3, 2025 19:20
@domi4484 domi4484 requested a review from gacarrillor April 3, 2025 19:20
Copy link
Collaborator

@gacarrillor gacarrillor left a comment

Choose a reason for hiding this comment

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

Nice addition!

It would be great to have tests one day for these important methods :)

action = self.create_action("help.png", "Aiuto", self.do_help)
self.toolbar.addAction(action)
self.iface.addPluginToMenu(PLUGIN_NAME, action)
settings_action = self.create_action("gear.png", "Ipostazioni", self.do_settings)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Italian typo 😛

(My level of Italian has suddenly increased by 0.0000001%)

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch thanks :)

@@ -61,6 +63,17 @@ def initAlgorithm(self, config=None):
)
)

self.addParameter(
QgsProcessingParameterField(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the value coming from a vector layer field or could we use QgsProcessingParameterNumber instead?

inLayer = self.parameterAsSource(parameters, self.INPUT, context)
mode = self.parameterAsEnum(parameters, self.MODE, context)
formFactor = self.parameterAsDouble(parameters, self.FORM_FACTOR, context)
areaTreshold = self.parameterAsDouble(parameters, self.AREA_TRESHOLD, context)
Copy link
Collaborator

Choose a reason for hiding this comment

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

*areaThreshold

if feedback.isCanceled():
break

print("Error: could not merge feature: {}".format(feature.id()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should avoid prints in Processing algorithms, we could use the feedback object instead.

def _checkArea(self, geometry, areaTreshold):
return geometry.area() <= areaTreshold

def _checkFormFactor(self, geometry, formFactorTreshold):
Copy link
Collaborator

Choose a reason for hiding this comment

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

*formFactorThreshold


# End while
if not processLayer.commitChanges():
raise QgsProcessingException(self.tr("Could not commit changes"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should call processLayer.rollBack() if we couldn't commit, specially because we'll use the dataProvider to add features in line 216, so we should have a clean layer buffer status before that line.

OUTPUT = "OUTPUT"
MODE = "MODE"
FORM_FACTOR = "FORM_FACTOR"
AREA_TRESHOLD = "AREA_TRESHOLD"
Copy link
Collaborator

Choose a reason for hiding this comment

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

*AREA_THRESHOLD

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.

None yet

2 participants