Opened 5 years ago

Last modified 5 years ago

#19667 needs_work enhancement

Export graphics objects to ipe

Reported by: etn40ff Owned by:
Priority: major Milestone: sage-6.10
Component: graphics Keywords: ipe, graphics
Cc: saliola, jason, was, chapoton, klee Merged in:
Authors: Salvatore Stella Reviewers:
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: u/etn40ff/matplotlib_ipe (Commits, GitHub, GitLab) Commit: d360f7a34c4c9bba6a7f6032edd62cccd0d2420e
Dependencies: Stopgaps:

Status badges

Description

This ticket includes https://github.com/otfried/ipe-tools/tree/master/matplotlib into matplotlib's spkg and add the option to save graphics objects as .ipe files to be edited with ipe http://ipe.otfried.org/

For some reason figure.tight_layout() fails with FigureCanvasIpe and I had to put it in the part of the code that runs conditionally.

Change History (12)

comment:1 Changed 5 years ago by chapoton

if you want a review, you should put the ticket to needs_review !

comment:2 Changed 5 years ago by etn40ff

  • Status changed from new to needs_review

Oops I forgot. Need review now S.

comment:3 Changed 5 years ago by chapoton

The doctest of save as ipe is not formatted correctly (missing :: and blank line after that)

There is a trivial failing doctest (see patchbot report)

But most importantly, changing matplotlib pkg without changing the version seems rather strange..

comment:4 Changed 5 years ago by git

  • Commit changed from a47c8f7584a69d188e49c5decf4a4169a501e020 to a799c2d42157ec62eea564640fc244d2bfeb2fa8

Branch pushed to git repo; I updated commit sha1. New commits:

7973eeaFixed failing doctest
5931513Properly formatted documentation
a799c2dChanged the way we patch matplotlib not to repackage the downloaded source

comment:5 Changed 5 years ago by git

  • Commit changed from a799c2d42157ec62eea564640fc244d2bfeb2fa8 to d360f7a34c4c9bba6a7f6032edd62cccd0d2420e

Branch pushed to git repo; I updated commit sha1. New commits:

d360f7aMissing blank line

comment:6 Changed 5 years ago by vdelecroix

Did you propose your enhancement to matplotlib? There is no point in making a patch that is not discussed upstream. If you did so, please fill the Report Upstream: field of the ticket and mention a link to the actual discussion.

comment:7 follow-up: Changed 5 years ago by etn40ff

  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

I had not thought of doing so beforehand because the patch is basically copy-paste from an ipe addons repository. I did report it upstream now, the relevant discussion should happen here: https://github.com/matplotlib/matplotlib/issues/5630

comment:8 in reply to: ↑ 7 Changed 5 years ago by vdelecroix

Replying to etn40ff:

I had not thought of doing so beforehand because the patch is basically copy-paste from an ipe addons repository. I did report it upstream now, the relevant discussion should happen here: https://github.com/matplotlib/matplotlib/issues/5630

If it is a copy-paste then:

  • mention from where it is copied
  • do you ask to the copied people whether they care about inclusion in matplotlib or Sage?

comment:9 Changed 5 years ago by etn40ff

Isn't the mention in SPKG.txt not enough as mentioning goes?

I did not contatct otfried (the developer of the file in question) yet but I did start a topic on this on matplotlib issue tracker; I am not at all affiliated with matplotlib so I think it is not my right to ask him about inclusion into matplotlib.

comment:10 Changed 5 years ago by nbruin

Is there a particular reason why we need to be able to export in a specific file format of a particular graphics editor? Usually editors will ensure they can read the main interchange formats (SVG would be the appropriate format here and a brief look at IPE suggests that it can import SVG files). There is a cost in adopting the code proposed here: the code needs to be maintained/ported/updated/documented/tested for as long as it remains in sage. What's the benefit if one could also just save and SVG file and import that into IPE for further processing?

(hint: briefly browsing the docs indicates that IPE7 cannot even read IPE6 files, so it doesn't seem the fileformat is very mature yet)

Last edited 5 years ago by nbruin (previous) (diff)

comment:11 Changed 5 years ago by etn40ff

As you said it should be the editor's job to be able to import from a standard format. This is not the case at the moment and I guess it will never be: if I understood correctly the idea is to keep the editor as minimal as possible.

I personally am interested in post-processing images produced by sage and, thinking this might be interesting for others, I made a patch. Of course this introduces a maintaining cost that we probably could avoid. On the other hand I do not see this feature being implemented upstream any time soon so this is the only reasonable option I could come up with. If you feel it goes beyond the guidelines of sage development I am ok with retiring this patch.

comment:12 Changed 5 years ago by chapoton

  • Status changed from needs_review to needs_work

does not longer apply

and putting it in "needs work" will help avoid to spend patchbot time here

Note: See TracTickets for help on using tickets.