#15609: IntegerVectors_nk.rank() specialization
-------------------------------------+-------------------------------------
Reporter: f.poloni | Owner:
Type: enhancement | Status: needs_work
Priority: trivial | Milestone: sage-6.1
Component: combinatorics | Resolution:
Keywords: integervectors, | Merged in:
rank | Reviewers:
Authors: Federico Poloni | Work issues:
Report Upstream: N/A | Commit:
Branch: | f616301fefa44762f90fe7cc54aab3d78d842b9e
u/f.poloni/ticket/15609 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by ncohen):
Hellooooooooooooooo !!!
> Thanks for taking a look at the patch. You are correct about the
negative entries, good catch; I'll fix it.
>
> Actually there is another point I was considering. The members of
`IntegerVectors` are lists; what should we do when another sequence type
such a tuple is passed to `rank()`?
Hmmmmmmm....
> The implementation in the current patch returns the same result for any
sequence type, however using `__contains__` would require them to be lists
or else it would throw an error.
Well, I think that it would make sense anyway to only return the rank of
objects on which `__contains__` return True. But indeed tuples make sense
too. Well, why don't we also allow tuples in `__contains__` ? It doesn't
look that crazy `:-)`
> `Combinations.rank()` throws `TypeError` when passed a tuple;
`Derangements.rank()` returns nothing (no error thrown).
I hate this code. This has to be *FIXED*. I will create a ticket for that
and add you in Cc (it should only take a couple of seconds to review).
What has to be known about this code is that it is inconsistent in many
places, and that we should never feel forced to respect what is done
elsewhere, for I learned it was mistakes most of the time `:-P`
I think rank should throw a `ValueError` when `__contains__` returns
`False` on the input. What's your opinion ? Returning nothing is just
plain bad programming.
> This is an issue of strict type correctness vs. user convenience. For
instance, you inadvertently used a tuple in your example in the previous
comment. `:)`
> It is ultimately a developer decision; please let me know what I should
return in case a tuple is entered and I shall code it.
Well, I really feel that what should be called there is `__contains__`,
and that we should play with what `__contains__` allow if it is not
convenient. But let's be consistent with ourselves for a start.
Please code it this way only if it makes sense to you. Otherwise let's
talk about it. And I will write this quick patch for the other problem you
found.
Have fuuuuuuuuuuuuun !
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/15609#comment:6>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
