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

Deep copy for packages and diagrams #2475

Merged
merged 10 commits into from
Jul 22, 2023
Merged

Deep copy for packages and diagrams #2475

merged 10 commits into from
Jul 22, 2023

Conversation

amolenaar
Copy link
Member

@amolenaar amolenaar commented Jul 14, 2023

PR Type

What kind of change does this PR introduce?

  • Bug fix
  • Feature
  • Chore (refactoring, formatting, local variables, other cleanup)
  • Documentation content changes

What is the current behavior?

Packages are only copied as is, without content.
Diagrams can be put on a diagram, and they can be copied. However, the contents of the diagram is not copied.

This is especially useful if you copy content between models.

Issue Number: #2422

What is the new behavior?

  • Elements are copied with their owned elements. So if you copy a package and do a full paste, the element and all owned elements are copied.
  • Diagrams copy the elements in the diagram. You can now copy a diagram (from a diagram) and full-paste it. This will result in a copy of the diagram
  • Update Modeling language docs with the (new) copy/paste interface and did some general improvements as well.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Cleaned up the copy and paste interfaces.

Copying is now done via the copy_full() function. This function copies the elements, and their owned elements.

NB. Copying is still supposed to happen from diagram to diagram. You cannot copy content in the model browser. This is something we can support in the future, but I deliberately kept it out of scope for this feature.

@github-actions github-actions bot added the python Pull requests that update Python code label Jul 14, 2023
@amolenaar amolenaar marked this pull request as draft July 14, 2023 06:37
@amolenaar amolenaar changed the title WIP: Deep copy for packages and diagrams Deep copy for packages and diagrams Jul 14, 2023
@amolenaar amolenaar linked an issue Jul 14, 2023 that may be closed by this pull request
This way `copy()` can focus on copying a single element with
mandatory related elements, while `copy_full()` copies whole hierarchies.
Now, if you do a full paste, the whole hierarchy is copied.

NB. Diagrams are not populated correctly at this point.
Use model linked to diagram by default.
You can now make a full copy of a diagram (from a diagram) and
paste it.

This is done by recording the original diagram id(s). When pasting
the original diagram ids are mapped to the new diagram. The Presentation
paste function does a lookup for the target diagram, which can now also
be a newly created diagram.
@amolenaar amolenaar marked this pull request as ready for review July 22, 2023 12:24
Place singledispatch functions together.
@amolenaar amolenaar requested a review from danyeaw July 22, 2023 13:03
@amolenaar amolenaar requested a review from sz332 July 22, 2023 15:50
@danyeaw danyeaw added feature A new feature and removed python Pull requests that update Python code documentation labels Jul 22, 2023
Copy link
Contributor

@sz332 sz332 left a comment

Choose a reason for hiding this comment

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

Tested with both class and activity diagrams, copy-paste between models works as expected: creates a copy of all elements as expected.

Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

This is a great upgrade to the copy/paste system, nice one @amolenaar!

@danyeaw danyeaw merged commit c18fbbb into main Jul 22, 2023
20 checks passed
@danyeaw danyeaw deleted the copy-diagrams branch July 22, 2023 17:50
@mikekidner
Copy link
Contributor

@amolenaar / @danyeaw this works great for elements. I noticed that when copying packages, the sub-packages are empty. Also, see how "Existing Package" takes on the name "New Package" in the new model. Not sure what I was expecting, perhaps that ExistingModel becomes a sub package of NewModel.

Screen Shot 2023-07-23 at 8 29 25 am

@mikekidner
Copy link
Contributor

In addition, this crash report after closing the above example may be relevant. I closed the new model without saving and then saved the existing model.

gaphor.plugins.errorreports.errorreports ERROR Uncaught exception
  + Exception Group Traceback (most recent call last):
  |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/ui/filemanager.py", line 462, in response
  |     handler(answer)
  |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/ui/filemanager.py", line 406, in response
  |     confirm_shutdown()
  |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/ui/filemanager.py", line 387, in confirm_shutdown
  |     self.event_manager.handle(SessionShutdown(self))
  |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/core/eventmanager.py", line 83, in handle
  |     self._events.handle(queue.pop())
  |   File "/Users/mikekidner/BitBucketRepos/gaphor/.venv/lib/python3.11/site-packages/generic/event.py", line 61, in handle
  |     raise ExceptionGroup("Error while handling events", exceptions)
  | ExceptionGroup: Error while handling events (1 sub-exception)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/.venv/lib/python3.11/site-packages/generic/event.py", line 57, in handle
    |     handler(event)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/application.py", line 115, in on_session_shutdown
    |     self.shutdown_session(session)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/application.py", line 143, in shutdown_session
    |     session.shutdown()
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/application.py", line 227, in shutdown
    |     self.shutdown_service(name, srv)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/application.py", line 234, in shutdown_service
    |     srv.shutdown()
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/core/modeling/elementfactory.py", line 76, in shutdown
    |     self.flush()
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/core/modeling/elementfactory.py", line 197, in flush
    |     element.unlink()
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/core/modeling/diagram.py", line 327, in unlink
    |     self.connections.remove_connections_to_item(item)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/.venv/lib/python3.11/site-packages/gaphas/connections.py", line 165, in remove_connections_to_item
    |     disconnect_item(*cinfo)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/.venv/lib/python3.11/site-packages/gaphas/connections.py", line 148, in _disconnect_item
    |     callback(item, handle, connected, port)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/diagram/_connector.py", line 108, in __call__
    |     adapter.disconnect(handle)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/UML/classes/containmentconnect.py", line 69, in disconnect
    |     oct.subject.package = owner_package(oct.diagram.owner)
    |     ^^^^^^^^^^^^^^^^^^^
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/core/modeling/element.py", line 214, in __setattr__
    |     super().__setattr__(key, value)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/core/modeling/properties.py", line 149, in __set__
    |     self.set(obj, value)
    |   File "/Users/mikekidner/BitBucketRepos/gaphor/gaphor/core/modeling/properties.py", line 381, in set
    |     raise TypeError(f"Can not set {obj}.{self.name} to itself")
    | TypeError: Can not set <gaphor.UML.uml.Package element b0d8bd42-2066-11ee-b860-ba28606445aa>.package to itself
    +------------------------------------
gaphor.storage.storage INFO Read 3 elements from file

@mikekidner
Copy link
Contributor

I couldn't get the crash to happen again. So maybe unrelated.

@amolenaar
Copy link
Member Author

Thanks for testing, @mikekidner. I'll have another look at copying to other models.

@amolenaar
Copy link
Member Author

It looks like our Hypothesis tests ran into the same issue you experienced: #2536 / https://github.com/gaphor/gaphor/actions/runs/5634866266/job/15265216692.

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

Successfully merging this pull request may close these issues.

Package import from file/clipboard
4 participants