# sympy/sympy

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

# QQ.algebraic_field(Rational) should be just QQ #14476

Open
opened this Issue Mar 11, 2018 · 9 comments

## Comments

Projects
None yet
3 participants
Contributor

### skirpichev commented Mar 11, 2018

 or raise some exception. Following doesn't have any sense: ```In [2]: QQ.algebraic_field(Rational(1, 7)) Out[2]: QQ<1/7>```
Member

### jksuom commented Mar 11, 2018

 I think `QQ.algebraic_field(r)` for a rational `r` (including 0 and 1) or a sequence of rational numbers should be `QQ`. Raising an exception is probably not necessary.
Contributor

### abaokar-07 commented Mar 11, 2018

 If the following change is made in `sympy/polys/numberfields.py` ``````if ex.is_Rational: raise SomeException("description") `````` then `QQ.algebraic_field(Rational(1, 7))` is also raising exception, but other examples are failing in good numbers
Contributor

### skirpichev commented Mar 11, 2018

 I think QQ.algebraic_field(r) for a rational r (including 0 and 1) or a sequence of rational numbers should be QQ. So, QQ in this context should be understood as degree=0 field extension of QQ itself? If the following change is made ... then Then don't do this. Proper fix in the way @jksuom suggested - should, probably, introduce `__new__` method for AlgebraicField.
Member

### jksuom commented Mar 11, 2018

 degree=1 field extension of QQ itself? That would be natural. `__new__` would probably be needed (unless RationalField could be made a subclass of AlgebraicField).
Contributor

### abaokar-07 commented Mar 11, 2018 • edited

 @jksuom @skirpichev What `__new__` should contain?
Contributor

### skirpichev commented Mar 11, 2018

 On Sun, Mar 11, 2018 at 03:13:30AM -0700, Kalevi Suominen wrote: unless RationalField could be made a subclass of AlgebraicField). Probably, in this case too. But I'm not sure that subclassing may have some mathematical sense in this case.
Member

### jksuom commented Mar 11, 2018

 Probably, in this case too. I agree. Though I think that RationalField could be a subclass of AlgebraicField (finite algebraic extension of degree 1) from the mathematical point of view. The situation is analogous to Integer being derived Rational (rational number with denominator 1).
Contributor

### abaokar-07 commented Mar 11, 2018

 So subclassing will be enough in this case?
Member

### jksuom commented Mar 11, 2018

 No, I don't think that is enough. But at some later point it may become useful.

### skirpichev added a commit to skirpichev/diofant that referenced this issue Jul 8, 2018

``` QQ.algebraic_field(Rational) == QQ ```
`Closes sympy/sympy#14476`
``` 76e1480 ```

### skirpichev added a commit to skirpichev/diofant that referenced this issue Jul 9, 2018

``` QQ.algebraic_field(Rational) == QQ ```
`Closes sympy/sympy#14476`
``` 3e555a2 ```
to join this conversation on GitHub. Already have an account? Sign in to comment