-
Notifications
You must be signed in to change notification settings - Fork 6
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 relationships between tables #1756
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will make large model analysis so much easier!
The current changes also will make the snapping + edge creation based on coordinates obsolete
The code for this is still there, with self.edge_layer.editingStopped.connect(self.connect_nodes)
. How do you mean obsolete? That people will get geometries from IDs rather than IDs from snapped geometries? But the geometries are not automatically created right? If I fill in just IDs in the windows from your screenshot, I get ValueError: Null geometry cannot be converted to a polyline.
.
This will need some updates of https://ribasim.org/tutorial/qgis.html and the test plan #1691 as well..
Mostly that we will get the IDs from the user, and not from the snapped geometries. We need to think about that workflow. Geometry creation is still needed, but maybe snap the first and last points to the chosen node_ids in the form? Anyway, that discussion is not blocking for this PR. I will update the documentation though. |
def set_unique(self, name: str) -> None: | ||
layer = self.layer | ||
index = layer.fields().indexFromName(name) | ||
setup = QgsEditorWidgetSetup( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to keep a separation between Qgis things (in the widgets folder), and core things that can be unit tested. It feels like this is being compromised with this new code as codecode also tells us there are no unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merely copied the existing structure here, the method above also uses an QgsEditorWidgetSetup. That said, I agree that the QGIS plugin can use some spring cleaning to separate concerns and document the architecture, as its a bit maddening to work with. Then again, I don't expect that writing C++ code in Python will get more fun.
Also, codecode?
rel.setReferencingLayer(from_layer.id()) | ||
rel.setReferencedLayer(to_layer_id) | ||
rel.setName(name) | ||
rel.setStrength(rel.RelationStrength.Composition) # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the type ignored in this case? Version mismatch between the stubs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stubs do not seem to be aware of enums linked in a class.
Fixes #1648
Fixes #1755 (by updating the edge style)
Fixes #1688 (node_id has to be unique)
Also sets the node_id and edge_id to be unique, so they are autogenerated in the edit forms.
The current changes also will make the snapping + edge creation based on coordinates obsolete, as the form now looks like this:
There are buttons to open a form with the Node, a highlighter to blink the node on the map, and a map selection panel, where one can click the desired node.