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:  sage6.3 
Component:  combinatorics  Keywords:  
Cc:  sagecombinat, 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 sagecombinat 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 followup: ↓ 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 copypaste.
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 sage6.2 to sage6.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