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 tscrim

  • Branch set to public/categories/change_an_element_with_realizations-21059
  • Commit set to f8a1bbb2cade40a8d21cc43d35db6ec5bd1c922f
  • Status changed from new to needs_review

New commits:

f8a1bbbUse a_realization() instead of realizations()[0].

comment:2 Changed 3 years ago by andrew.mathas

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...

Version 1, edited 3 years ago by andrew.mathas (previous) (next) (diff)

comment:3 Changed 3 years ago by tscrim

Are these are all fixed by #21077?

comment:4 Changed 3 years ago by hivert

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 hivert

  • Reviewers set to Florent Hivert
  • Status changed from needs_review to positive_review

comment:6 Changed 3 years ago by hivert

  • Keywords days85 added

comment:7 Changed 3 years ago by vbraun

  • Branch changed from public/categories/change_an_element_with_realizations-21059 to f8a1bbb2cade40a8d21cc43d35db6ec5bd1c922f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.