Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dir(RIF) contains duplicates #13043

Closed
saraedum opened this issue May 28, 2012 · 12 comments
Closed

dir(RIF) contains duplicates #13043

saraedum opened this issue May 28, 2012 · 12 comments

Comments

@saraedum
Copy link
Member

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.

Component: categories

Keywords: beginner sd40.5

Author: Julian Rueth

Reviewer: Mike Hansen

Merged: sage-5.1.beta2

Issue created by migration from https://trac.sagemath.org/ticket/13043

@saraedum
Copy link
Member Author

Changed keywords from sd40.5 to beginner sd40.5

@sagetrac-dsm
Copy link
Mannequin

sagetrac-dsm mannequin commented May 28, 2012

comment:2

I don't seem to see the patch.

@saraedum
Copy link
Member Author

comment:3

Attachment: trac_13043.patch.gz

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.

@saraedum
Copy link
Member Author

Author: Julian Rueth

@roed314
Copy link
Contributor

roed314 commented May 28, 2012

comment:5

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).

@mwhansen
Copy link
Contributor

comment:6

Replying to @roed314:

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?

@roed314
Copy link
Contributor

roed314 commented May 28, 2012

comment:7

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.

@mwhansen
Copy link
Contributor

comment:8

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

@roed314
Copy link
Contributor

roed314 commented May 28, 2012

comment:9

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

@mwhansen
Copy link
Contributor

Reviewer: Mike Hansen

@mwhansen
Copy link
Contributor

comment:10

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

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2012

Merged: sage-5.1.beta2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants