Opened 3 years ago
Closed 3 years ago
#21059 closed defect (fixed)
Change WithRealizations._an_element_ to use a_realization
Reported by: | tscrim | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.3 |
Component: | categories | Keywords: | days85 |
Cc: | ntheiry, darij | Merged in: | |
Authors: | Travis Scrimshaw | Reviewers: | Florent Hivert |
Report Upstream: | N/A | Work issues: | |
Branch: | f8a1bbb (Commits) | Commit: | f8a1bbb2cade40a8d21cc43d35db6ec5bd1c922f |
Dependencies: | Stopgaps: |
Description
As noticed on #21054, there is a discrepancy between one
and _an_element_
for parents that are in the category *.WithRealizations
, in that the former uses a_realization
, whereas the latter uses realizations()[0]
. While on #21054, this did uncover an error, this cases problems with the test suite when a realization does not get created before an_element
gets called (which I have noticed before).
Thus, I propose to have _an_element_
use a_realization()
, which is a required implementation and _an_element_
will also have consistent output no matter which basis is created first.
Change History (7)
comment:1 Changed 3 years ago by
- Branch set to public/categories/change_an_element_with_realizations-21059
- Commit set to f8a1bbb2cade40a8d21cc43d35db6ec5bd1c922f
- Status changed from new to needs_review
comment:2 Changed 3 years ago by
There are two failing doc-tests in sage/combinat/kazhdan_lusztig
when the optional coxeter3 package is installed.
1 item had failures: 2 of 8 in sage.combinat.kazhdan_lusztig.KazhdanLusztigPolynomial [26 tests, 2 failures, 2.76 s] ---------------------------------------------------------------------- sage -t kazhdan_lusztig.py # 2 doctests failed ----------------------------------------------------------------------
and more failures in sage/libs/coxeter3/
.
---------------------------------------------------------------------- sage -t coxeter3/coxeter.pyx # 256 doctests failed sage -t coxeter3/coxeter_group.py # 120 doctests failed ----------------------------------------------------------------------
Not entirely sure what the policy on doc-tests in optional packages...
comment:3 Changed 3 years ago by
Are these are all fixed by #21077?
comment:4 Changed 3 years ago by
Except for rebasing and doctests this looks good to me. I'll handle the rebasing shortly and put a positive review.
comment:5 Changed 3 years ago by
- Reviewers set to Florent Hivert
- Status changed from needs_review to positive_review
comment:6 Changed 3 years ago by
- Keywords days85 added
comment:7 Changed 3 years ago by
- Branch changed from public/categories/change_an_element_with_realizations-21059 to f8a1bbb2cade40a8d21cc43d35db6ec5bd1c922f
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Use a_realization() instead of realizations()[0].