Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change WithRealizations._an_element_ to use a_realization #21059

Closed
tscrim opened this issue Jul 19, 2016 · 10 comments
Closed

Change WithRealizations._an_element_ to use a_realization #21059

tscrim opened this issue Jul 19, 2016 · 10 comments

Comments

@tscrim
Copy link
Collaborator

tscrim commented Jul 19, 2016

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.

CC: @nthiery @darijgr

Component: categories

Keywords: days85

Author: Travis Scrimshaw

Branch: f8a1bbb

Reviewer: Florent Hivert

Issue created by migration from https://trac.sagemath.org/ticket/21059

@tscrim tscrim added this to the sage-7.3 milestone Jul 19, 2016
@tscrim
Copy link
Collaborator Author

tscrim commented Jul 19, 2016

New commits:

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

@tscrim
Copy link
Collaborator Author

tscrim commented Jul 19, 2016

@tscrim
Copy link
Collaborator Author

tscrim commented Jul 19, 2016

Commit: f8a1bbb

@AndrewMathas
Copy link
Member

comment:2

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 is on doc-tests in optional packages...

@tscrim
Copy link
Collaborator Author

tscrim commented Jul 27, 2016

comment:3

Are these are all fixed by #21077?

@hivert
Copy link

hivert commented Mar 14, 2017

comment:4

Except for rebasing and doctests this looks good to me. I'll handle the rebasing shortly and put a positive review.

@hivert
Copy link

hivert commented Mar 14, 2017

Reviewer: Florent Hivert

@hivert
Copy link

hivert commented Mar 14, 2017

Changed keywords from none to days85

@vbraun
Copy link
Member

vbraun commented Mar 27, 2017

@fchapoton
Copy link
Contributor

Changed commit from f8a1bbb to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants