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.

Sign up for GitHub

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

Jump to bottom

Deprecate sage.structure.graphics_file #32988

Closed
mkoeppe opened this issue Dec 7, 2021 · 11 comments
Closed

Deprecate sage.structure.graphics_file #32988

mkoeppe opened this issue Dec 7, 2021 · 11 comments
Labels
p: minor / 4 t: enhancement t: refactoring
Milestone
sage-9.6

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 7, 2021

This module appears to be an unused leftover of the old ipython notebook customized to Sage.

CC: @kcrisman @kwankyu @vbraun

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: e783e4e

Reviewer: Kwankyu Lee, Karl-Dieter Crisman

Issue created by migration from https://trac.sagemath.org/ticket/32988
@mkoeppe mkoeppe added this to the sage-9.5 milestone Dec 7, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 9, 2021

Branch: u/mkoeppe/deprecate_sage_structure_graphics_file

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 9, 2021

Author: Mattthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 9, 2021

New commits:

e783e4esrc/sage/structure/graphics_file.py: Deprecate

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 9, 2021

Commit: e783e4e

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 10, 2021

Changed author from Mattthias Koeppe to Matthias Koeppe

@kcrisman
Copy link
Member

comment:5

Huh. I have no input here - you may very well be right, but I don't recall messing with this particular module in that context. However, this log seems to indicate it actually originates from trying to make the old ipython notebook work by trying to recognize files that should actually be displayed - perhaps that is still necessary for Jupyter (new or old or future)? Looks like Volker wrote it. On a related note, did you try launch viewer in the command line to see what happens (if anything)?

Of course, all of that is just a bunch of questions; if it really isn't used any more, this would seem pretty reasonable to remove.

@kwankyu
Copy link
Collaborator

kwankyu commented Dec 14, 2021

comment:6

That file was added in #16996, to support graphics output in the pre-jupyter era ipython notebook. It seems that as Sage went over to use jupyter rich-output facilities, the GraphicsFile class provided by the file became out of use. The function graphics_from_save also seems to have become the method of the same name of the display manager defined in sage.repl.rich_output.

Hence I am positive to deprecate this file.

@kwankyu

This comment has been minimized.

Sign in to view
@kwankyu
Copy link
Collaborator

kwankyu commented Dec 15, 2021

Reviewer: Kwankyu Lee, Karl-Dieter Crisman

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 15, 2021

comment:8

Thanks!

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Jan 11, 2022
@vbraun
Copy link
Member

vbraun commented Feb 16, 2022

Changed branch from u/mkoeppe/deprecate_sage_structure_graphics_file to e783e4e

@vbraun vbraun closed this as completed in 69ea2e5 Feb 16, 2022
mkoeppe added a commit to mkoeppe/sage that referenced this issue Sep 19, 2023
@mkoeppe
sage.media, sage.structure.graphics_file: Remove; deprecated in sagem…
3f2a1d5 
…ath#12673 (2021), sagemath#32988 (2022)
vbraun pushed a commit to vbraun/sage that referenced this issue Sep 23, 2023
sagemathgh-36305: sage.media, sage.structure.graphics_file: Remov…
d7404b1 
…e; deprecated in sagemath#12673, sagemath#32988

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36305
Reported by: Matthias Köppe
Reviewer(s):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
p: minor / 4 t: enhancement t: refactoring
Projects
None yet
Milestone
sage-9.6
Development

No branches or pull requests
4 participants
@vbraun @kwankyu @kcrisman @mkoeppe