Opened 8 years ago
Closed 8 years ago
#15195 closed defect (fixed)
Family cardinality does not catch enough errors
Reported by: | tscrim | Owned by: | tscrim |
---|---|---|---|
Priority: | major | Milestone: | sage-5.13 |
Component: | combinatorics | Keywords: | Family cardinality |
Cc: | nthiery | Merged in: | sage-5.13.beta2 |
Authors: | Travis Scrimshaw | Reviewers: | Nathann Cohen |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
We have the following:
sage: C = CartesianProduct(PositiveIntegers(), [1,2,3]) sage: C.cardinality() +Infinity sage: F = Family(C, lambda x: x) sage: F.cardinality() Throws a TypeError
The problem is that the cardinality()
tries to run len()
on an infinite set, which throws a TypeError
. Fixes this by also catching type errors.
Attachments (1)
Change History (7)
comment:1 Changed 8 years ago by
- Description modified (diff)
- Status changed from new to needs_review
Changed 8 years ago by
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
- Cc nthiery added
Its hard for me to say. If it typically gets python objects like list
, then it will be faster. If it's expecting objects with a cardinality()
method, then it's the reverse.
Nicolas, which do you think should be attempted first (note that this is for LazyFamily
)?
Best,
Travis
comment:4 Changed 8 years ago by
- Reviewers set to Nathann Cohen
- Status changed from needs_review to positive_review
Okay okay, if it's about comparing the different use-cases then let's merge this patch first then think about that.
Especially if obtaining an answer takes three days :-P
Nathann
comment:5 Changed 8 years ago by
Thanks Nathann.
Travis
comment:6 Changed 8 years ago by
- Merged in set to sage-5.13.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
Hmmm... It fixes the bug indeed, but why wouldn't we do the opposite and first try to call
.set.cardinality()
and onAttributeError
run thelen
version ? Is this a speed problem ?Nathann