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.

Sign up for GitHub

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

Jump to bottom

remove bad is_subcategory implementation #13930

Closed
nbruin opened this issue Jan 8, 2013 · 6 comments
Closed

remove bad is_subcategory implementation #13930

nbruin opened this issue Jan 8, 2013 · 6 comments
Assignees
@nthiery
Labels
c: categories p: major / 3 t: bug
Milestone
sage-5.6

Comments

@nbruin
Copy link
Contributor

nbruin commented Jan 8, 2013

As discovered on #13400, there is a bad implementation of is_subcategory in categories/covariant_functorial_construction.py: It doesn't cache although it should, it doesn't have a proper docstring and it shadows a better implementation on a superclass. It simply needs to be removed.

CC: @simon-king-jena

Component: categories

Author: Nils Bruin

Reviewer: Simon King

Merged: sage-5.6.rc0

Issue created by migration from https://trac.sagemath.org/ticket/13930
@nbruin nbruin added this to the sage-5.6 milestone Jan 8, 2013
@nbruin
Copy link
Contributor Author

nbruin commented Jan 8, 2013

Attachment: trac_13930-del_is_subcat.patch.gz

Straightforward fix

@nbruin
Copy link
Contributor Author

nbruin commented Jan 8, 2013

Author: Nils Bruin

@nbruin
Copy link
Contributor Author

nbruin commented Jan 9, 2013

comment:3

Simon and Nicholas would be very qualified to review this.

@simon-king-jena
Copy link
Member

comment:4

We have discussed on #13400 that the removal if this is_subcat method makes sense, the change is indirectly doctested by the fact that the existing tests don't break. Hence, the patch seems fine to me.

The patchbot is happy, and I am happy as well.

@simon-king-jena
Copy link
Member

Reviewer: Simon King

@jdemeyer
Copy link

Merged: sage-5.6.rc0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nthiery nthiery

Labels
c: categories p: major / 3 t: bug
Projects
None yet
Milestone
sage-5.6
Development

No branches or pull requests
4 participants
@nthiery @nbruin @jdemeyer @simon-king-jena