Skip to content

Conversation

@mantepse
Copy link
Contributor

The removed line was marked as dead by codecov in #38821. For safety, we remove it in a separate pull request.

Dependencies: #38821

@fchapoton
Copy link
Contributor

I am not quite sure this is "dead code". Untested code for sure, ja.

@github-actions
Copy link

github-actions bot commented Oct 29, 2024

Documentation preview for this PR (built with commit 3119816; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mantepse
Copy link
Contributor Author

Indeed, you are right. Should we make this a doctest?

from sage.structure.element import Element

class MyRingElement(Element):
    """
    sage: R = MyRing()
    sage: R.ideal(R(1))
    """
    def __init__(self, parent, x):
        Element.__init__(self, parent)
        self._x = parent._K(x)

    def _add_(self, other):
        P = self.parent()
        return P.element_class(P, self._x + other._x)

    def _mul_(self, other):
        return P.element_class(P, self._x * other._x)

class MyRing(Parent):
    def __init__(self):
        R.<x,y> = PolynomialRing(QQ)
        Q = R.quotient(x^2+y^2+1)
        self._K = Q
        category = PrincipalIdealDomains()
        Parent.__init__(self, base=QQ, category=category)

    Element = MyRingElement

@mantepse
Copy link
Contributor Author

Should we (i.e., I) add this as a test to categories.rings?

@mantepse mantepse force-pushed the ideal_to_cat_remove_dead_code branch from 4a4f592 to cf7c8d1 Compare October 31, 2024 09:10
@fchapoton
Copy link
Contributor

I am not so sure. Maybe we can think about that again once the big changes are merged ?

@mantepse
Copy link
Contributor Author

I am not so sure. Maybe we can think about that again once the big changes are merged ?

Yes, of course! Note that the tests show that the code is not so useful for principal ideal domains without gcd.

@tscrim
Copy link
Collaborator

tscrim commented Nov 7, 2024

#38821 has now been merged I believe.

@tscrim
Copy link
Collaborator

tscrim commented Oct 8, 2025

There are doctest failures.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants