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

[ENH] Implement copying graph to clipboard using Ctrl-C (Cmd-C) #1386

Merged
merged 1 commit into from
Jun 30, 2016

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Jun 26, 2016

This should work for all widgets that have graph_name attribute (~ the Save Image button).

Can somebody check it on Linux and Windows? Scatter plot and Box plot, for instance.

@VesnaT, you're an expert for Linear Projection - and saving graphs in general. Can you check why it copying doesn't work for Linear Projection and Distributions?

@codecov-io
Copy link

codecov-io commented Jun 26, 2016

Current coverage is 87.73%

Merging #1386 into master will not change coverage

@@             master      #1386   diff @@
==========================================
  Files            75         75          
  Lines          7421       7421          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           6511       6511          
  Misses          910        910          
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by 3df2437...55d31c1

@@ -198,6 +199,9 @@ def __new__(cls, *args, **kwargs):
sc = QShortcut(QKeySequence(Qt.ShiftModifier | Qt.Key_F1), self)
sc.activated.connect(self.__quicktip)

sc = QShortcut(QKeySequence(Qt.ControlModifier | Qt.Key_C), self)
Copy link
Contributor

Choose a reason for hiding this comment

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

QKeySequence(QKeySequence.Copy)

@ajdapretnar ajdapretnar self-assigned this Jun 27, 2016
@kernc
Copy link
Contributor

kernc commented Jun 27, 2016

Currently segfaults on GNU/Linux.

graph_obj = getdeepattr(self, self.graph_name, None)
if graph_obj is None:
return
ClipboardFormat.write_image(None, graph_obj)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't step through it after here. 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With which visualization? All? Perhaps you should ask @VesnaT. Linear projection report used to fail on Os X in a similar call, and she fixed it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. Doesn't segfault anymore, yet nothing has changed code-wise. But anyway, now the images are borked:
distributions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you try now? For me it now works on all visualizations.

Copy link
Contributor

@kernc kernc Jun 28, 2016

Choose a reason for hiding this comment

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

Works here too. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can somebody (@thocevar?) try this PR on Windows?

@janezd janezd force-pushed the graph-to-clipboard branch 2 times, most recently from 81607eb to c5df407 Compare June 28, 2016 18:23
@ajdapretnar
Copy link
Contributor

@janezd Tested it. Works on all visualizations, apart from Classification Tree Viewer, which I reckon is of a different type. It also works on Word Cloud in Text add-on, but not on GeoMap. Networks don't work either. But the rest work like a charm, so it's up to you to decide whether we patch those as well or not. 😄 Still, I'm over the moon with this one! :)

@janezd
Copy link
Contributor Author

janezd commented Jun 28, 2016

I fixed tree view. If this is the only one that didn't work, we're done here.

As for add-ons, GeoMap does not seem so trivial. Networks should be simple, but if I apply the same fix as for trees, the points and edges are misaligned.

@janezd janezd changed the title [WIP] [ENH] Implement copying graph to clipboard using Ctrl-C (Cmd-C) [ENH] Implement copying graph to clipboard using Ctrl-C (Cmd-C) Jun 28, 2016
@@ -551,3 +551,6 @@ def save_graph(self):
save_plot(data=dict(scene=self.scene, tree=self.tree),
file_formats=dict(chain(FileFormat.img_writers.items(),
FileFormat.graph_writers.items())))

def copy_to_clipboard(self):
ClipboardFormat.write_image(None, self.scene)
Copy link
Contributor

@kernc kernc Jun 28, 2016

Choose a reason for hiding this comment

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

Might setting OWTreeViewer2D.graph_name = 'scene' above also work?

@kernc
Copy link
Contributor

kernc commented Jun 28, 2016

Both are pretty trivial. Just a matter of assigning OWNxExplorer.graph_name = 'view' and OWGeoMap.graph_name = 'webview'. Works for me.

@janezd
Copy link
Contributor Author

janezd commented Jun 29, 2016

I changed the tree widget as suggested.

For networks, the problem was that I had an old version of the widget. With the last version, OWNxExplorer.graph_name works for me, too. But it's in add-on, so it doesn't belong to this PR.

@kernc
Copy link
Contributor

kernc commented Jun 29, 2016

It appears the latest changes haven't been pushed yet?

@janezd
Copy link
Contributor Author

janezd commented Jun 30, 2016

Sorry, I forgot. I pushed them now.

@kernc kernc merged commit 44bccc5 into biolab:master Jun 30, 2016
@janezd janezd deleted the graph-to-clipboard branch July 1, 2016 15:22
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.

4 participants