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

py3 making __nonzero__ an alias for __bool__ in rings folder #21887

Closed
fchapoton opened this issue Nov 17, 2016 · 20 comments
Closed

py3 making __nonzero__ an alias for __bool__ in rings folder #21887

fchapoton opened this issue Nov 17, 2016 · 20 comments

Comments

@fchapoton
Copy link
Contributor

as a step to py3

part of #16076

here only in the rings folder

Component: python3

Author: Frédéric Chapoton

Branch/Commit: b279834

Reviewer: Jeroen Demeyer

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

@fchapoton fchapoton added this to the sage-7.5 milestone Nov 17, 2016
@fchapoton
Copy link
Contributor Author

New commits:

1eb9e08making `__nonzero__` an alias for `__bool__` in rings folder

@fchapoton
Copy link
Contributor Author

Commit: 1eb9e08

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/21887

@jdemeyer
Copy link

comment:2

This is not needed for Cython extension types. Cython understands both special methods __nonzero__ and __bool__.

There is one potential caveat: if you really need __nonzero__ and __bool__ non-special methods (that is, you really need obj.__bool__ and not just bool(obj) or not obj), your trick might be needed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

e62142dtrac 21887 one missing case

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Changed commit from 1eb9e08 to e62142d

@fchapoton
Copy link
Contributor Author

comment:4

So should I refrain to make changes to any pyx or pxi files ?

I would prefer to have something uniform, even if it is not necessary. Does it do any harm to do that also in pyx files ?

@jdemeyer
Copy link

comment:5

Replying to @fchapoton:

Does it do any harm to do that also in pyx files ?

There is no obvious harm, apart from code bloat.

@jdemeyer
Copy link

comment:6

OK, I actually tested it and it doesn't work...

/usr/local/src/sage-config/src/sage/rings/complex_number.pyx in init sage.rings.complex_number (/usr/local/src/sage-config/src/build/cythonized/sage/rings/complex_number.c:25899)()
    860                                       imag_string, digit_precision_bound)
    861 
    862     def __bool__(self):
    863         """
    864         Return ``True`` if ``self`` is not zero. This is an internal function;
    865         use :meth:`is_zero()` instead.
    866 
    867         EXAMPLES::
    868 
    869             sage: z = 1 + CC(I)
    870             sage: z.is_zero()
    871             False
    872         """
    873         return not (mpfr_zero_p(self.__re) and mpfr_zero_p(self.__im))
    874 
--> 875     __nonzero__ =__bool__
        global __nonzero__ = undefined
        global __bool__ = undefined
    876 
    877     def prec(self):
    878         """
    879         Return precision of this complex number.
    880 
    881         EXAMPLES::
    882 
    883             sage: i = ComplexField(2000).0
    884             sage: i.prec()
    885             2000
    886         """
    887         return self._parent.prec()
    888 
    889     def real(self):
    890         """

NameError: name '__bool__' is not defined

I guess Cython extension types don't allow such assignment of special methods.

@jdemeyer
Copy link

comment:7

To fix code like obj.__nonzero__() (for example in src/sage/rings/complex_interval.pyx), I would just use bool(obj). That should "just work" in Python 2 and Python 3.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

41d63abtrac 21887 undoing alias of `__nonzero__` in pyx files

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Changed commit from e62142d to 41d63ab

@jdemeyer
Copy link

comment:9

.pxi files should be treated just like .pyx files.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

28728dftrac 21887 undo aliasing in pxi files too

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Changed commit from 41d63ab to 28728df

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Changed commit from 28728df to b279834

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 17, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

b279834trac 21887 some forgotten cases

@fchapoton
Copy link
Contributor Author

comment:12

ok, works now, green bot, setting to needs_review

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Nov 19, 2016

Changed branch from u/chapoton/21887 to b279834

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

3 participants