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

ascii_art fail in jupyter notebook #33996

Closed
kwankyu opened this issue Jun 15, 2022 · 19 comments
Closed

ascii_art fail in jupyter notebook #33996

kwankyu opened this issue Jun 15, 2022 · 19 comments

Comments

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 15, 2022

When running sage in Jupyter notebook,

sage: ascii_art(list(Partitions(5)))
<repr(<sage.typeset.ascii_art.AsciiArt at 0x7f8f358aa250>) failed: OSError: [Errno 25] Inappropriate ioctl for device>

This is because the _terminal_width() method does not work with Jupiter.

Component: user interface

Author: Kwankyu Lee

Branch/Commit: 5aa8cd5

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/33996

@kwankyu kwankyu added this to the sage-9.7 milestone Jun 15, 2022
@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

New commits:

ed22f90Replace fileno() hack with isatty()

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

Commit: ed22f90

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

Branch: u/klee/33996

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

Author: Kwankyu Lee

@dimpase
Copy link
Member

dimpase commented Jun 15, 2022

comment:4

This removed try/except block. It's not clear why it was there in the 1st place, but would it be easier to keep it?

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

comment:5

Replying to @dimpase:

This removed try/except block. It's not clear why it was there in the 1st place, but would it be easier to keep it?

It is explained in the comment. Not necessary anymore.

@dimpase
Copy link
Member

dimpase commented Jun 15, 2022

comment:6

There could still be cases we don't know about, e.g. emacs mode, the modes used by one of these VSCode plugins, etc...

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

comment:7

Replying to @dimpase:

There could still be cases we don't know about, e.g. emacs mode, the modes used by one of these VSCode plugins, etc...

According to the comment, the author of the code, Volker, only considered terminal and IPython. So the try/except code is only for those two cases.

There is no reason to suspect that this change will make the situation with other user interfaces worse or better... In any case, it could be dealt with in other tickets if reported by users of those interfaces.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

comment:8

Replying to @kwankyu:

In any case, it could be dealt with in other tickets if reported by users of those interfaces.

Or in this ticket. We can wait.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

comment:9

Replying to @kwankyu:

There is no reason to suspect that this change will make the situation with other user interfaces worse or better...

If any other user interface is affected, then it would be rather because of replacing os.isatty(sys.stdout.fileno()) with sys.stdout.isatty().

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

comment:10

I put a post in sage-devel: https://groups.google.com/g/sage-devel/c/EHCPeH5BXh4

@dimpase
Copy link
Member

dimpase commented Jun 15, 2022

comment:11

to copy my sage-dev message:


I think this fix is an API change. We have no control over what fake
ttys Sage users are using.
What earlier returned False might now lead to an error.
So if you want to remove that try/except, you must deprecate it first.

IMHO I'd better have try/except retained (your fix applied in the body of try)

@dimpase
Copy link
Member

dimpase commented Jun 15, 2022

comment:12

By the way, I checked that emacs-mode works with this change. Another tricky case might be sage cell.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 15, 2022

Changed commit from ed22f90 to 5aa8cd5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 15, 2022

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

5aa8cd5Retain try/except clause

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jun 15, 2022

comment:14

Replying to @dimpase:

IMHO I'd better have try/except retained (your fix applied in the body of try)

Okay. Being cautious doesn't hurt. Thanks.

@dimpase
Copy link
Member

dimpase commented Jun 15, 2022

comment:15

OK, good.

@dimpase
Copy link
Member

dimpase commented Jun 15, 2022

Reviewer: Dima Pasechnik

@vbraun
Copy link
Member

vbraun commented Jun 28, 2022

Changed branch from u/klee/33996 to 5aa8cd5

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

No branches or pull requests

3 participants