Opened 8 years ago

Closed 8 years ago

#13043 closed defect (fixed)

dir(RIF) contains duplicates

Reported by: saraedum Owned by: nthiery
Priority: trivial Milestone: sage-5.1
Component: categories Keywords: beginner sd40.5
Cc: Merged in: sage-5.1.beta2
Authors: Julian Rueth Reviewers: Mike Hansen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Currently, dir(RIF), and probably a few others, contains duplicates:

sage: A = dir(RIF)
sage: B = set(A)
sage: for b in B: A.remove(b)
sage: A
['__doc__', '_sage_src_lines_', 'is_field', 'is_integrally_closed']

The attached patch fixes this problem.

Attachments (1)

trac_13043.patch (1.4 KB) - added by saraedum 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by saraedum

  • Keywords changed from beginner, sd40.5 to beginner sd40.5

comment:2 Changed 8 years ago by dsm

I don't seem to see the patch.

Changed 8 years ago by saraedum

comment:3 Changed 8 years ago by saraedum

hehe, you've been too fast. I needed the ticket number first to mention it in the TESTS section. I'm running the full doctests now, after that I'll set it to needs review; but feel free to have a look already.

comment:4 Changed 8 years ago by saraedum

  • Authors set to Julian Rueth
  • Status changed from new to needs_review

comment:5 follow-up: Changed 8 years ago by roed

The problem with using the previous approach was that various inputs to unique_merge weren't sorted. Rather than this fix I would prefer fixing dir(self.class) to return a sorted list (currently 'Hom' is first), and to sort self.dict.keys() before passing it to unique_merge (note that all of the duplicated keys in this case come from self.dict.keys(), since dir(cls) is almost sorted).

comment:6 in reply to: ↑ 5 Changed 8 years ago by mhansen

Replying to roed:

Rather than this fix I would prefer fixing dir(self.class) to return a sorted list (currently 'Hom' is first), and to sort self.dict.keys() before passing it to unique_merge (note that all of the duplicated keys in this case come from self.dict.keys(), since dir(cls) is almost sorted).

Is this just for performance reasons?

comment:7 Changed 8 years ago by roed

Yeah. But I didn't mark it as needs work since I wanted another opinion. And since I haven't compared the performance of the two options.

comment:8 Changed 8 years ago by mhansen

Is dir() actually used in a performance critical spot? I had just assumed it was basically for introspection.

comment:9 Changed 8 years ago by roed

It probably is just for introspection. If someone else wants to give Julian's solution a positive review I'll be totally happy. :-)

comment:10 Changed 8 years ago by mhansen

  • Reviewers set to Mike Hansen
  • Status changed from needs_review to positive_review

I looked at it and think that it's fine.

comment:11 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.1.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.