Opened 11 years ago

Closed 9 years ago

#12958 closed defect (wontfix)

Fix sageinspect to slightly better support old style classes

Reported by: Nicolas M. Thiéry Owned by: Minh Van Nguyen
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: documentation Keywords: sageinspect
Cc: Sage Combinat CC user, Simon King 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 Nicolas M. Thiéry)

See the patch.

Attachments (1)

trac_12958-sage_inspect_for_classes-nt.patch (1.6 KB) - added by Nicolas M. Thiéry 11 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 years ago by Simon King

What attached fix?

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

comment:2 Changed 11 years ago by Nicolas M. Thiéry

Status: newneeds_review
Summary: Fix sageinspect to better support classes with metaclasses.Fix sageinspect to better support old style classes

comment:3 in reply to:  1 Changed 11 years ago by Nicolas M. Thiéry

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,

Changed 11 years ago by Nicolas M. Thiéry

comment:4 Changed 11 years ago by Simon King

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 11 years ago by Nicolas M. Thiéry

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 11 years ago by Nicolas M. Thiéry

Description: modified (diff)
Summary: Fix sageinspect to better support old style classesFix sageinspect to slightly better support old style classes

comment:7 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:8 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:9 Changed 9 years ago by Ralf Stephan

Milestone: sage-6.2sage-duplicate/invalid/wontfix
Reviewers: Ralf Stephan
Status: needs_reviewpositive_review

Appears to be no longer relevant.

comment:10 Changed 9 years ago by Volker Braun

Resolution: wontfix
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.