Opened 9 years ago

Closed 8 years ago

Last modified 5 years ago

#15246 closed enhancement (fixed)

add xkcd function to sage

Reported by: Maarten Derickx Owned by:
Priority: minor Milestone: sage-6.4
Component: user interface Keywords:
Cc: Merged in:
Authors: Maarten Derickx, André Apitzsch Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: dfc8753 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

Sage already has an xgcd function but no xkcd function. We should really change that.

Attachments (1)

15246-xkcd.patch (2.4 KB) - added by Maarten Derickx 9 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 Changed 9 years ago by Maarten Derickx

Component: number fieldsnumber theory
Status: newneeds_review

comment:2 Changed 9 years ago by aapitzsch

Status: needs_reviewneeds_work

Use a python 3 compatible syntax to raise errors

raise RuntimeError("Could not obtain comic data from %s . Maybe you should enable time travel!"%url)

Changed 9 years ago by Maarten Derickx

Attachment: 15246-xkcd.patch added

comment:3 Changed 9 years ago by Maarten Derickx

Status: needs_workneeds_review

comment:4 Changed 9 years ago by Maarten Derickx

Ok, I changed the patch according to your comment.

comment:5 Changed 9 years ago by For batch modifications

Milestone: sage-6.0sage-6.1

comment:6 Changed 9 years ago by Frédéric Chapoton

Milestone: sage-6.1sage-duplicate/invalid/wontfix
Reviewers: Frédéric Chapoton
Status: needs_reviewpositive_review

Useless. Sage is not the place for that.

comment:7 Changed 9 years ago by Nils Bruin

Status: positive_reviewneeds_review

Reassigning a ticket to "won't fix" and giving the positive review by the same person is bad form. I happen to think it's a fun easter egg. Xkcd often does have relevant and highly quotable mathematics-related cartoons.

comment:8 Changed 9 years ago by Frédéric Chapoton

Ok.

Sorry for misbehaving.

I still think that this has nothing to do here, and that kids should go and play elsewhere.

comment:9 Changed 9 years ago by Frédéric Chapoton

Reviewers: Frédéric Chapoton

comment:10 Changed 9 years ago by Kiran Kedlaya

I happen to think there is a valid reason to insert this easter egg: it might earn Sage a mention in a future xkcd comic, which would be excellent marketing. I can even imagine a "Droste effect" where one of the characters uses Sage, mistypes the xgcd command and sees... him/herself.

My one complaint is the use of an NSFW doctest. Surely we can be less immature than that! (This reminds me of the discovery of "B00B135" in some shipped Microsoft code. Not helpful.)

comment:11 in reply to:  10 Changed 9 years ago by Karl-Dieter Crisman

I happen to think there is a valid reason to insert this easter egg: it might earn Sage a mention in a future xkcd comic, which would be excellent marketing. I can even imagine a "Droste effect" where one of the characters uses Sage, mistypes the xgcd command and sees... him/herself.

Given that the author has already been interviewed in Math Horizons, I think this is a definite possibility.

My one complaint is the use of an NSFW doctest. Surely we can be less immature than that! (This reminds me of the discovery of "B00B135" in some shipped Microsoft code. Not helpful.)

Hmm, very good point. Although the specific comic itself is probably one of the greatest ones out there for "pure" math types (though I was amazed at the haters out there for this one). Maybe we can use the ... in the title attribute of the img link? Or do you have a different comic suggestion? I looked a few just now but the rest of the better math-specific ones are (of course) somewhat more sub-discipline specific.

comment:12 Changed 9 years ago by Karl-Dieter Crisman

Component: number theoryuser interface
Priority: majorminor

However, let's move it to the correct component :)

comment:13 Changed 9 years ago by Maarten Derickx

So this ticket seems that this ticket is getting some attention again. I would like to adres the comments above so that this ticket can get a positive review and get merged.

However before this the question: should this ticket be "won't fix" or should it be given a mile stone again in order to get merged still has to be settled. Right now there only seems to be one person that thinks that this should be "won't fix". So I would say we should give it a mile stone again. But it doesn't feel like good behavior to do that myself as I am the author of this ticket.

comment:14 Changed 9 years ago by Kiran Kedlaya

Milestone: sage-duplicate/invalid/wontfixsage-6.1

Sorry, I had meant to change the milestone back but I forgot.

comment:15 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:16 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:17 Changed 8 years ago by aapitzsch

Authors: Maarten Derickx, André Apitzsch
Branch: u/aapitzsch/ticket/15246
Commit: 9743e619e17a8ff40682261f9908b5b5d1e4f2dd

comment:18 Changed 8 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work
  1. Please remove the alt text as requested by Kedlaya.
  1. Never use a bare except:, always use except Exception: or even better, catch a specific exception.
  1. Can you avoid the finally: file.close() using a with: statement?

comment:19 Changed 8 years ago by Kiran Kedlaya

On further reflection, I suppose we should still doctest alt text. Maybe use comic 353 (Python) for that instead?

comment:20 Changed 8 years ago by git

Commit: 9743e619e17a8ff40682261f9908b5b5d1e4f2dd9f39fa108ec6d0bda1db8265d93563ee8fdb72a2

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

b78a7aaMerge remote-tracking branch 'origin/develop' into xkcd
9f39fa1reviewer comments

comment:21 Changed 8 years ago by aapitzsch

Status: needs_workneeds_review

comment:22 Changed 8 years ago by Nils Bruin

Rebase perhaps? Given that the original doctest was considered unprofessional, perhaps we do not want it to become part of the history of sage?

comment:23 in reply to:  22 Changed 8 years ago by aapitzsch

Branch: u/aapitzsch/ticket/15246u/aapitzsch/ticket/15246_xkcd
Commit: 9f39fa108ec6d0bda1db8265d93563ee8fdb72a2b7cd4584da0fbe3436a2e0fd2dc396182353affa

Replying to nbruin:

Rebase perhaps? Given that the original doctest was considered unprofessional, perhaps we do not want it to become part of the history of sage?

Done.

comment:24 Changed 8 years ago by Jeroen Demeyer

Branch: u/aapitzsch/ticket/15246_xkcdu/jdemeyer/ticket/15246
Created: Oct 1, 2013, 1:39:27 PMOct 1, 2013, 1:39:27 PM
Modified: Aug 3, 2014, 7:56:17 PMAug 3, 2014, 7:56:17 PM

comment:25 Changed 8 years ago by Jeroen Demeyer

Commit: b7cd4584da0fbe3436a2e0fd2dc396182353affadfc8753829cc5fcf256a6698b94206fb50de797e

New commits:

dfc8753Use xkcd comic number from json data

comment:26 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:27 Changed 8 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

LGTM (unless it's easy to open a web browser only in the command line a la import antigravity).

comment:28 Changed 8 years ago by Volker Braun

Branch: u/jdemeyer/ticket/15246dfc8753829cc5fcf256a6698b94206fb50de797e
Resolution: fixed
Status: positive_reviewclosed

comment:29 Changed 8 years ago by Nathann Cohen

Commit: dfc8753829cc5fcf256a6698b94206fb50de797e

AT LAST !

Nathann

comment:30 Changed 8 years ago by Nathann Cohen

HMmmm... Looks like sending this message erased the "commit" field, but I can't set it back O_o

Nathann

comment:31 Changed 5 years ago by Karl-Dieter Crisman

See #23348 as a followup.

Note: See TracTickets for help on using tickets.