#9235: Doctest coverage for sage.categories.homset
-----------------------------+----------------------------------------------
Reporter: SimonKing | Owner: nthiery
Type: defect | Status: needs_work
Priority: minor | Milestone: sage-4.7.1
Component: categories | Keywords: doctest coverage homset
Work_issues: | Upstream: N/A
Reviewer: Niles Johnson | Author: Simon King
Merged: | Dependencies:
-----------------------------+----------------------------------------------
Changes (by niles):
* status: needs_review => needs_work
* reviewer: => Niles Johnson
Old description:
> The doctest coverage for sage.categories.homset was
> {{{
> SCORE devel/sage-main/sage/categories/homset.py: 52% (13 of 25)
> }}}
>
> My patch covers all but two methods:
>
> * get_action_c
> * coerce_map_from_c
>
> These two return (by default) None. Is there a good ''indirect'' doctest
> for these two? I am not familiar with {{{get_action}}}, and I don't know
> what {{{coerce_map_from_c}}} does, compared with {{{_coerce_map_from_}}}.
> Perhaps the reviewer can explain it to me, so that I or s/he can add the
> two missing tests?
New description:
The doctest coverage for sage.categories.homset was
{{{
SCORE devel/sage-main/sage/categories/homset.py: 52% (13 of 25)
}}}
My patch covers all but two methods:
* get_action_c
* coerce_map_from_c
These two return (by default) None. Is there a good ''indirect'' doctest
for these two? I am not familiar with {{{get_action}}}, and I don't know
what {{{coerce_map_from_c}}} does, compared with {{{_coerce_map_from_}}}.
Perhaps the reviewer can explain it to me, so that I or s/he can add the
two missing tests?
--
Comment:
You're right -- this shouldn't have to wait so long! I've looked through
all the changes, and they look good! All tests pass, and the
documentation builds cleanly, without warnings.
I used `search_src` to look for other places in Sage where `get_action_c`
and `coerce_map_from_c` are used. The only places they appear are in
`structure/parent_old`, so I think these in Homset should be deprecated
and (later) removed. I've added a reviewer patch which adds deprecation
warnings and corresponding tests, raising the coverage to 100%.
The only issue I have with `9235_doctest_homset.patch` is the following
block:
{{{
if category is None:
if cat_X.is_subcategory(cat_Y):
category = cat_Y
elif cat_Y.is_subcategory(cat_X):
# NT: this "category is None" test is useless and could be
removed
# SK: Indeed! For that reason, the ValueError would never be
raised
# NT: why is there an assymmetry between X and Y?
# SK: I see no reason. In particular, I don't see why an error
should
# be raised if cat_X is not cat_Y. So, I uncomment the
following
# two lines.
## if not (category is None) and not (cat_X is cat_Y):
## raise ValueError, "No unambiguous category found for
Hom from %s to %s."%(X,Y)
category = cat_X
else:
# Search for the lowest common super category
subcats_X = cat_X.all_super_categories(proper = True)
subcats_Y = set(cat_Y.all_super_categories(proper = True))
category = None
for c in subcats_X:
if c in subcats_Y:
category = c
break
if category is None:
raise TypeError, "No suitable category found for Hom from
%s to %s."%(X,Y)
}}}
If there's no reason to include the second "`category is None`" test, then
it and the previous comments should simply be deleted. And there is a
third "`category is None`" test in this block which also looks redundant.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9235#comment:7>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
