Opened 9 years ago
Closed 9 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 9 years ago by
Authors:  → Nicolas M. Thiéry 

Cc:  sagecombinat ncohen vdelecroix added 
Component:  PLEASE CHANGE → combinatorics 
Description:  modified (diff) 
Type:  PLEASE CHANGE → defect 
comment:2 Changed 9 years ago by
Summary:  Fix FiniteEnumeratedSet's call to accept Python objects as input → Fix call for FiniteEnumeratedSet over plain Python objects 

comment:3 Changed 9 years ago by
Branch:  → u/nthiery/fix_call_for_finite_enumeratedset_s_over_plain_python_objects 

comment:4 Changed 9 years ago by
Commit:  → 8ac32c2d6854519a96eb1ce261240a0ee8537cb3 

Description:  modified (diff) 
Status:  new → needs_review 
Summary:  Fix call for FiniteEnumeratedSet over plain Python objects → Fix call for FiniteEnumeratedSet's of plain Python objects 
comment:5 Changed 9 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 9 years ago by
Status:  needs_review → needs_work 

comment:7 Changed 9 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 9 years ago by
Commit:  8ac32c2d6854519a96eb1ce261240a0ee8537cb3 → 4e274542d24211d57e9f6d99562c56385235721a 

Branch pushed to git repo; I updated commit sha1. New commits:
4e27454  #16280: Trivial doctest fixes

comment:11 Changed 9 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 9 years ago by
Status:  needs_work → needs_review 

comment:13 Changed 9 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 9 years ago by
Reviewers:  → Nathann Cohen 

Status:  needs_review → positive_review 
comment:15 Changed 9 years ago by
Milestone:  sage6.2 → sage6.3 

comment:16 Changed 9 years ago by
Branch:  u/nthiery/fix_call_for_finite_enumeratedset_s_over_plain_python_objects → 4e274542d24211d57e9f6d99562c56385235721a 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
#16280: Fix call for FiniteEnumeratedSet's of plain Python objects