Opened 3 years ago

Closed 3 years ago

#24350 closed enhancement (fixed)

SageObject.__repr__(): fall back to base class

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.2
Component: misc Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: fc4461d (Commits, GitHub, GitLab) Commit: fc4461d21d277c5616df36cf4f32aadb5b7ba454
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

SageObject.__repr__(self) tries to call self._repr_(). If that method does not exist, it confusingly falls back to returning str(type(self)). Instead, it would be better to fall back to super().__repr__().

Change History (16)

comment:1 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/sageobject___repr______fall_back_to_base_class

comment:2 Changed 3 years ago by jdemeyer

  • Commit set to 4dc09b4a7831ea07088639a718f1354fed266cae
  • Status changed from new to needs_review

New commits:

​4dc09b4SageObject.__repr__(): fall back to base class

comment:3 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 3 years ago by git

  • Commit changed from 4dc09b4a7831ea07088639a718f1354fed266cae to 4de00b1b641795e64e7247d67550c69f3c304c76

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​4de00b1SageObject.__repr__(): fall back to base class

comment:5 follow-up: Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

It took me a moment to realize why the current behavior is wrong (or at least highly misleading), but once I did, this is definitely what should be done. Positive review if the patchbot comes back green.

comment:6 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:7 in reply to: ↑ 5 Changed 3 years ago by jdemeyer

Replying to tscrim:

It took me a moment to realize why the current behavior is wrong (or at least highly misleading), but once I did, this is definitely what should be done.

Indeed. It's also the kind of thing where you wonder why nobody noticed this before...

comment:8 Changed 3 years ago by jdemeyer

The old behaviour dates back to 

commit 0cd022a9fac32d648b25b59564428355c2fa395f
Author: William Stein <wstein@ucsd.edu>
Date:   Tue Jun 6 21:59:47 2006 +0000

    [project @ notebook -- first alpha version of the new SAGE notebook: ]
    type notebook() at the SAGE prompt to try it out!

comment:9 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:10 Changed 3 years ago by git

  • Commit changed from 4de00b1b641795e64e7247d67550c69f3c304c76 to 120e4e366fd8327bb159ea4def3ac47bb5d1710d

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

​120e4e3Merge tag '8.2.beta0' into t/24350/sageobject___repr______fall_back_to_base_class

comment:11 Changed 3 years ago by git

  • Commit changed from 120e4e366fd8327bb159ea4def3ac47bb5d1710d to ee717d3abf04b2dcce59c270b344b28f21df92e7

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

​ee717d3Trivial doctest fix

comment:12 Changed 3 years ago by git

  • Commit changed from ee717d3abf04b2dcce59c270b344b28f21df92e7 to fc4461d21d277c5616df36cf4f32aadb5b7ba454

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​fc4461dFix a few doctests

comment:13 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

I accidentally hard-coded the Sage version number in a few doctests. Last commit needs review.

comment:14 Changed 3 years ago by jdemeyer

Patchbot is green.

comment:15 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:16 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/sageobject___repr______fall_back_to_base_class to fc4461d21d277c5616df36cf4f32aadb5b7ba454
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.