Opened 7 months ago
Closed 6 months 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 tscrim)
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 7 months ago by tscrim
- Description modified (diff)
- Status changed from new to needs_review
Changed 7 months ago by tscrim
comment:2 Changed 6 months ago by ncohen
comment:3 Changed 6 months ago by tscrim
- 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 6 months ago by ncohen
- 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 6 months ago by tscrim
Thanks Nathann.
Travis
comment:6 Changed 6 months ago by jdemeyer
- 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 on AttributeError run the len version ? Is this a speed problem ?
Nathann