Opened 7 years ago
Closed 7 years ago
#16280 closed defect (fixed)
Fix call for FiniteEnumeratedSet's of plain Python objects
Reported by: | nthiery | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.3 |
Component: | combinatorics | Keywords: | |
Cc: | sage-combinat, ncohen, vdelecroix | Merged in: | |
Authors: | Nicolas M. Thiéry | Reviewers: | Nathann Cohen |
Report Upstream: | N/A | Work issues: | |
Branch: | 4e27454 (Commits, GitHub, GitLab) | Commit: | 4e274542d24211d57e9f6d99562c56385235721a |
Dependencies: | Stopgaps: |
Description (last modified by )
*Problem*
Before this ticket, a FiniteEnumeratedSet
could not be called on its
own elements whenever they were not Element
's:
sage: F = FiniteEnumeratedSet(["a", 1]) sage: F(1) 1 sage: F("a") ... TypeError: Cannot convert str to sage.structure.element.Element
This prevented the use of F(x)
as generic idiom to convert x
into
F
while checking that it's in F
.
And indeed:
sage: TestSuite(F).run() Failure in _test_an_element: ... TypeError: Cannot convert str to sage.structure.element.Element ------------------------------------------------------------ The following tests failed: _test_an_element
*Analysis*
Parent.__call__
enforces that _element_constructor_
return an
Element
(more precisely, it calls _element_constructor_
through a
DefaultConvertMap
, and any Map
requires its results to be
instances of Element
).
*Proposed solution*
Since FiniteEnumeratedSets
is often a facade over plain Python
objects, this ticket works around this limitation by a custom
FiniteEnumeratedSets.__call__
that calls directly
_element_constructor_
whenever el
is not an
Element
. Otherwise
Parent.__call__
is called as usual.
*Limitation*
This workaround prevents conversions or coercions from facade parents over plain Python objects. But it's already much better than before!
Change History (16)
comment:1 Changed 7 years ago by
- Cc sage-combinat ncohen vdelecroix added
- Component changed from PLEASE CHANGE to combinatorics
- Description modified (diff)
- Type changed from PLEASE CHANGE to defect
comment:2 Changed 7 years ago by
- Summary changed from Fix FiniteEnumeratedSet's call to accept Python objects as input to Fix call for FiniteEnumeratedSet over plain Python objects
comment:3 Changed 7 years ago by
- Branch set to u/nthiery/fix_call_for_finite_enumeratedset_s_over_plain_python_objects
comment:4 Changed 7 years ago by
- Commit set to 8ac32c2d6854519a96eb1ce261240a0ee8537cb3
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from Fix call for FiniteEnumeratedSet over plain Python objects to Fix call for FiniteEnumeratedSet's of plain Python objects
comment:5 Changed 7 years ago by
sage -t --long sets/totally_ordered_finite_set.py # 1 doctest failed sage -t --long sets/finite_enumerated_set.py # 3 doctests failed
Nothing serious, but there is something weird O_O;;
File "sets/finite_enumerated_set.py", line 345, in sage.sets.finite_enumerated_set.FiniteEnumeratedSet.__call__ Failed example: psi('a') Expected: 2 Got: 1
comment:6 Changed 7 years ago by
- Status changed from needs_review to needs_work
comment:7 Changed 7 years ago by
Oh, I did not try to pass test in all of src/. My office's computer is off for some reason ^^;
Nathann
comment:8 Changed 7 years ago by
- Commit changed from 8ac32c2d6854519a96eb1ce261240a0ee8537cb3 to 4e274542d24211d57e9f6d99562c56385235721a
Branch pushed to git repo; I updated commit sha1. New commits:
4e27454 | #16280: Trivial doctest fixes
|
comment:9 Changed 7 years ago by
Running all long tests now. I'll report around noon.
comment:10 follow-up: ↓ 11 Changed 7 years ago by
Do you get why this 2
changed to a 1
? O_o
comment:11 in reply to: ↑ 10 Changed 7 years ago by
Replying to ncohen:
Do you get why this
2
changed to a1
?O_o
It was meant to be a one in the first place: it's the length of 'a'; I had fumbled my copy-paste.
Btw: all long tests pass.
comment:12 Changed 7 years ago by
- Status changed from needs_work to needs_review
comment:13 Changed 7 years ago by
Oh sorry, I had not noticed that you had added this doctest with '2' as an answer yourself. I was wondering how your commit could have changed a former "2" into a "1" :-D
Good to go as far as I can tell. Seems to do what it should, and if it passes tests... Thanks ! Now #16269 can be reviewed, which means that #16277 can be reviewed which means that #16279 can be reviwed, but also #16235 and so #16241.
Damn designs :-PPPPPPPPP
Nathann
comment:14 Changed 7 years ago by
- Reviewers set to Nathann Cohen
- Status changed from needs_review to positive_review
comment:15 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:16 Changed 7 years ago by
- Branch changed from u/nthiery/fix_call_for_finite_enumeratedset_s_over_plain_python_objects to 4e274542d24211d57e9f6d99562c56385235721a
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
#16280: Fix call for FiniteEnumeratedSet's of plain Python objects