Opened 10 years ago
Closed 10 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)
Change History (12)
comment:1 Changed 10 years ago by
- Keywords changed from beginner, sd40.5 to beginner sd40.5
comment:2 Changed 10 years ago by
Changed 10 years ago by
comment:3 Changed 10 years ago by
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 10 years ago by
- Status changed from new to needs_review
comment:5 follow-up: ↓ 6 Changed 10 years ago by
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 10 years ago by
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 10 years ago by
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 10 years ago by
Is dir() actually used in a performance critical spot? I had just assumed it was basically for introspection.
comment:9 Changed 10 years ago by
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 10 years ago by
- 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 10 years ago by
- Merged in set to sage-5.1.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
I don't seem to see the patch.