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

Change name of misc/sagex_ds.pyx #5160

Closed
kcrisman opened this issue Feb 2, 2009 · 10 comments
Closed

Change name of misc/sagex_ds.pyx #5160

kcrisman opened this issue Feb 2, 2009 · 10 comments

Comments

@kcrisman
Copy link
Member

kcrisman commented Feb 2, 2009

Since #5094 is merged, I'll open this ticket. It seems to be a worthwhile idea.

Rename sage/misc/sagex_ds.pyx to sage/misc/binary_tree.pyx, which also conveys much better what this file is about. Also fix some doctest formatting, and add the random tester.

Component: documentation

Author: Jeroen Demeyer

Reviewer: Karl-Dieter Crisman

Merged: sage-5.4.rc0

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

@kcrisman kcrisman added this to the sage-5.4 milestone Feb 2, 2009
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Feb 2, 2009

comment:1

The deprecation policy might apply here.

Cheers,

Michael

@jdemeyer
Copy link

Attachment: 5160_rename_sagex_ds.patch.gz

@jdemeyer
Copy link

Author: Jeroen Demeyer

@jdemeyer

This comment has been minimized.

@kcrisman
Copy link
Member Author

comment:3

Nice. I'll try to review it if someone else doesn't get there first. Do you think we'd need a deprecation period, or is it unlikely anyone would actually use this other than in the files in question? Who could we ask?

@jdemeyer
Copy link

comment:4

Replying to @kcrisman:

Nice. I'll try to review it if someone else doesn't get there first. Do you think we'd need a deprecation period, or is it unlikely anyone would actually use this other than in the files in question?

I think it's unlikely, since it's barely used in Sage itself. Besides, Sage code shouldn't be affected:

sage: BinaryTree()

should work just as before.

I don't even know how to deprecate a module name (as opposed to a function).

@kcrisman
Copy link
Member Author

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member Author

comment:5

I don't even know how to deprecate a module name (as opposed to a function).

Good point, which is why I didn't do it before.

This looks good. I guess at some point this was supposed to have a lot more than binary trees :) Running irrelevant tests now, but the relevant ones were fine. I'm getting some errors, but they seem unrelated - I'll look into it.

@kcrisman
Copy link
Member Author

comment:6

Yeah, they seem to be unrelated - weird, but I must have done something wrong. They occurred with and without the patch.

@jdemeyer
Copy link

Merged: sage-5.4.rc0

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

2 participants