Opened 10 years ago

Closed 8 years ago

#12958 closed defect (wontfix)

Fix sageinspect to slightly better support old style classes

Reported by: nthiery Owned by: mvngu
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: documentation Keywords: sageinspect
Cc: sage-combinat, SimonKing Merged in:
Authors: Nicolas M. Thiéry Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by nthiery)

See the patch.

Attachments (1)

trac_12958-sage_inspect_for_classes-nt.patch (1.6 KB) - added by nthiery 10 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 follow-up: Changed 10 years ago by SimonKing

What attached fix?

Could you verify that what you intend to attach is not a duplicate of #11791?

comment:2 Changed 10 years ago by nthiery

  • Status changed from new to needs_review
  • Summary changed from Fix sageinspect to better support classes with metaclasses. to Fix sageinspect to better support old style classes

comment:3 in reply to: ↑ 1 Changed 10 years ago by nthiery

Replying to SimonKing:

What attached fix?

There it is; I decided at the last minute to add some doctests.

Could you verify that what you intend to attach is not a duplicate of #11791?

I actually fixed the title since it's more about old style classes. If the tests I put in the patch pass, I am happy to have this ticket closed as dup of #11791.

Cheers,

comment:4 follow-up: Changed 10 years ago by SimonKing

I am not sure what the two doc tests are supposed to demonstrate, as they just test against errors being raised.

Anyway, with #11791 (plus dependencies plus something more, so, someone else should cross verify), I get

sage: from sage.misc.sageinspect import sage_getsourcelines
sage: class Foo: pass
....: 
sage: sage_getsourcelines(Foo)  # Is it intended to fix this?
Traceback (most recent call last):
...
IOError: could not find class definition
sage: import inspect 
sage: inspect.findsource(Sets.ParentMethods) # Why is Python's inspect module tested??
Traceback (most recent call last):
...
IOError: could not find class definition
sage: L = sage_getsourcelines(Sets.ParentMethods)  # works just fine
sage: print ''.join(L[0])
    class ParentMethods:
#         # currently overriden by the default implementation in sage.structure.Parent
#         def __call__(self, *args, **options):
#             return self.element_class(*args, **options)

        # Todo: simplify the _element_constructor definition logic
...
            """
            if category is None:
                category = self.category()
            from sage.combinat.free_module import CombinatorialFreeModule
            return CombinatorialFreeModule(base_ring, self, category = category.Algebras(base_ring))

The code line that is changed in your patch is not touched by #11791. So, how does it actually change things?

comment:5 in reply to: ↑ 4 Changed 10 years ago by nthiery

Replying to SimonKing:

I am not sure what the two doc tests are supposed to demonstrate, as they just test against errors being raised.

The error is worst without the patch :-)

Anyway, with #11791 (plus dependencies plus something more, so, someone else should cross verify), I get

sage: from sage.misc.sageinspect import sage_getsourcelines
sage: class Foo: pass
....: 
sage: sage_getsourcelines(Foo)  # Is it intended to fix this?
Traceback (most recent call last):
...
IOError: could not find class definition
sage: import inspect 
sage: inspect.findsource(Sets.ParentMethods) # Why is Python's inspect module tested??
Traceback (most recent call last):
...
IOError: could not find class definition
sage: L = sage_getsourcelines(Sets.ParentMethods)  # works just fine

Cool.

The code line that is changed in your patch is not touched by #11791. So, how does it actually change things?

From the brief look I had at your path, the line is still there, but the logic has changed a bit before. I don't know, and honestly I don't want to spend more time on this. I guess I'll simply change #12953 to simply not test introspection too deeply since it will get better soon. Then it won't depend on this ticket and/or #11971.

Feel free to mark this one as a dup if you want!

Cheers,

Nicolas

comment:6 Changed 10 years ago by nthiery

  • Description modified (diff)
  • Summary changed from Fix sageinspect to better support old style classes to Fix sageinspect to slightly better support old style classes

comment:7 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:8 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 8 years ago by rws

  • Milestone changed from sage-6.2 to sage-duplicate/invalid/wontfix
  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

Appears to be no longer relevant.

comment:10 Changed 8 years ago by vbraun

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.