solving type error and Coercion error in algebraicfield.py #18669

mohitacecode · 2020-02-16T16:34:11Z

References to other Issues or PRs

Fixes #18248
Fixes #13230

Brief description of what is fixed or changed

It solves the type error by:

Adding a check for type of ext in AlgebraicField.__init__.

It also solves the Coercion error by:

adding a AlgebraicField.from_AlgebraicField(K1, a, K0) method.

Other comments

Release Notes

polys
- Fixed initialization of orig_ext and added from_AlgebraicField() to AlgebraicField.

sympy-bot · 2020-02-16T16:34:12Z

✅

Hi, I am the SymPy bot (v151). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

polys
- Fixed initialization of orig_ext and added from_AlgebraicField() to AlgebraicField. (#18669 by @mohitacecode)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

Fixes #18248
Fixes #13230
#### Brief description of what is fixed or changed
It solves the type error by:

- Adding a check for type of `ext` in `AlgebraicField.__init__`.

It also solves the Coercion error by:

- adding a `AlgebraicField.from_AlgebraicField(K1, a, K0)` method.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
*   polys
     *  Fixed initialization of `orig_ext` and added `from_AlgebraicField()` to `AlgebraicField`.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

czgdp1807 · 2020-02-16T16:51:11Z

sympy/polys/domains/algebraicfield.py

@@ -26,8 +26,10 @@ def __init__(self, dom, *ext):
            raise DomainError("ground domain must be a rational field")

        from sympy.polys.numberfields import to_number_field
-
-        self.orig_ext = ext
+        if len(ext) == 1 and type(ext[0]) is tuple:


if len(ext) == 1 and isinstance(ext[0], tuple)

Sure updating it now.
thanks for pointing it out.

codecov · 2020-02-16T19:02:18Z

Codecov Report

Merging #18669 into master will decrease coverage by 4.271%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##            master   #18669       +/-   ##
============================================
- Coverage   75.572%    71.3%   -4.272%     
============================================
  Files          644      644               
  Lines       167515   167831      +316     
  Branches     39485    39566       +81     
============================================
- Hits        126595   119665     -6930     
- Misses       35395    42519     +7124     
- Partials      5525     5647      +122

mohitacecode · 2020-02-17T09:17:40Z

I think it is ready @jksuom

jksuom · 2020-02-17T09:30:45Z

sympy/polys/domains/algebraicfield.py

@@ -124,3 +126,8 @@ def numer(self, a):
    def denom(self, a):
        """Returns denominator of ``a``. """
        return self.one
+
+    def from_AlgebraicField(K1, a, K0):
+        """Convert a polynomial to ``dtype``. """


Technically, a is represented by a polynomial but it is less confusing to say that it is an algebraic field element. So this should say something about converting an element a from one algebraic field to another.

Convert AlgebraicField element 'a' to another AlgebraicField element
Does it seems ok?

jksuom · 2020-02-17T09:31:36Z

sympy/polys/domains/algebraicfield.py

+    def from_AlgebraicField(K1, a, K0):
+        """Convert a polynomial to ``dtype``. """
+        a = K0.to_sympy(a)
+        return K1.from_sympy(a)


It is not necessary to save K0.to_sympy(a) temporarily to a local variable, K1.from_sympy(K0.to_sympy(a)) should suffice.

Yes I agree I will update it.

jksuom · 2020-02-17T09:33:45Z

There are a couple of comments, otherwise this looks good.

jksuom · 2020-02-17T17:38:18Z

What about the intersection of two circles in #13230? It would be good to have a test.

mohitacecode · 2020-02-17T17:41:22Z

Sure that should be checked too I will add it now.

mohitacecode · 2020-02-17T17:47:26Z

What about the intersection of two circles in #13230? It would be good to have a test.

The test for this should be added in test_ellipse.py. Is it okay to add here?

jksuom · 2020-02-17T18:25:56Z

Both of these issues run solvers but, actually, they are related to algebraic fields. Perhaps they should be added to test_numberfields.py.

mohitacecode · 2020-02-17T18:35:18Z

I just noticed this:
The o/p seems bit absurd to me. what do you think?

>>> c1 = Circle(Point2D(180 + 24*sqrt(210), 168), 231)
>>> c2 = Circle(Point2D(889, 89 + 2*sqrt(9879)), 174)
>>> intersection(c1, c2)
[Point2D((-1418*sqrt(9879)/381721 - 144*sqrt(230510)/381721 + 1896*sqrt(210)/381721 + 56011/381721)*(-38811853369687970538*sqrt(210)/507642891246375390197 + 5572625558444725932*sqrt(230510)/507642891246375390197 + sqrt(5)*sqrt(-2477733817168523132880172154355338155165662648305 + 94596782533875822530882841399346430904131232*sqrt(230510) + 680989928789592059191366739742910325258787060*sqrt(9879) + 169190923751886106340201853200012760065401726272*sqrt(210))/10152857824927507803940 + 544540497818290517582*sqrt(9879)/507642891246375390197 + 257193210521585085426053/2030571564985501560788) + 144*sqrt(230510)/4289 + 1418*sqrt(9879)/4289 + 5088228*sqrt(210)/381721 + 438049031/763442, -38811853369687970538*sqrt(210)/507642891246375390197 + 5572625558444725932*sqrt(230510)/507642891246375390197 + sqrt(5)*sqrt(-2477733817168523132880172154355338155165662648305 + 94596782533875822530882841399346430904131232*sqrt(230510) + 680989928789592059191366739742910325258787060*sqrt(9879) + 169190923751886106340201853200012760065401726272*sqrt(210))/10152857824927507803940 + 544540497818290517582*sqrt(9879)/507642891246375390197 + 257193210521585085426053/2030571564985501560788), Point2D((-1418*sqrt(9879)/381721 - 144*sqrt(230510)/381721 + 1896*sqrt(210)/381721 + 56011/381721)*(-sqrt(5)*sqrt(-2477733817168523132880172154355338155165662648305 + 94596782533875822530882841399346430904131232*sqrt(230510) + 680989928789592059191366739742910325258787060*sqrt(9879) + 169190923751886106340201853200012760065401726272*sqrt(210))/10152857824927507803940 - 38811853369687970538*sqrt(210)/507642891246375390197 + 5572625558444725932*sqrt(230510)/507642891246375390197 + 544540497818290517582*sqrt(9879)/507642891246375390197 + 257193210521585085426053/2030571564985501560788) + 144*sqrt(230510)/4289 + 1418*sqrt(9879)/4289 + 5088228*sqrt(210)/381721 + 438049031/763442, -sqrt(5)*sqrt(-2477733817168523132880172154355338155165662648305 + 94596782533875822530882841399346430904131232*sqrt(230510) + 680989928789592059191366739742910325258787060*sqrt(9879) + 169190923751886106340201853200012760065401726272*sqrt(210))/10152857824927507803940 - 38811853369687970538*sqrt(210)/507642891246375390197 + 5572625558444725932*sqrt(230510)/507642891246375390197 + 544540497818290517582*sqrt(9879)/507642891246375390197 + 257193210521585085426053/2030571564985501560788)

jksuom · 2020-02-17T18:52:54Z

There seems to be only one (real) point in the intersection. Can you check that by computing the distance between the center points? (That should be the sum or difference of the radii.)

mohitacecode · 2020-02-17T19:13:48Z

the distance between their center is around ~458.

mohitacecode · 2020-02-17T19:22:27Z

The answer (o/p in current branch) is giving two points so I think that circles are not only touching externally so the property (distance between the center points should be the sum or difference of the radii.) will not work here.

The reason I find it absurd because the answer seems too unsimplified and long to mee but now I think that answer can be unsimplified and long because of the ugly circles equation we have used here.

jksuom · 2020-02-17T19:29:38Z

The distance I get is 380.55 which is less than 231 + 174 = 405. So there should be two points.

mohitacecode · 2020-02-17T19:35:23Z

Yes distance between centre is 380.55 I had just by mistake took the points
(180+24sqft(210),6) instead of (180+24sqft(210),168).

mohitacecode · 2020-02-17T19:35:50Z

So the answer seems right I will add the test.

jksuom · 2020-02-18T16:38:34Z

sympy/polys/tests/test_numberfields.py

+def test_issue_13230():
+    c1 = Circle(Point2D(3, sqrt(5)), 5)
+    c2 = Circle(Point2D(4, sqrt(7)), 6)
+    assert intersection(c1, c2) == [Point2D(-1 + (-sqrt(7) + sqrt(5))*(-2*sqrt(7)/29 + 9*sqrt(5)/29 + sqrt(196*sqrt(35) + 1941)/29), -2*sqrt(7)/29 + 9*sqrt(5)/29 + sqrt(196*sqrt(35) + 1941)/29), Point2D(-1 + (-sqrt(7) + sqrt(5))*(-sqrt(196*sqrt(35) + 1941)/29 - 2*sqrt(7)/29 + 9*sqrt(5)/29), -sqrt(196*sqrt(35) + 1941)/29 - 2*sqrt(7)/29 + 9*sqrt(5)/29)]


This should be split onto several lines of length at most 72

sure I will update it.

mohitacecode · 2020-02-18T18:31:16Z

I have done the changes please see if seems ok to you.

jksuom · 2020-02-18T19:23:07Z

Thanks, looks good.

mohitacecode · 2020-02-18T19:25:11Z

Thanks :)

Iqrar99 · 2020-02-23T04:05:06Z

Hello @mohitacecode , I just want to know. How are you testing the test files? are you using coverage python or what? thanks.

mohitacecode · 2020-02-23T04:42:34Z

Hello @mohitacecode , I just want to know. How are you testing the test files? are you using coverage python or what? thanks.

You can see how to run all type of test here:
https://github.com/sympy/sympy/wiki/Running-tests

Iqrar99 · 2020-02-23T04:44:36Z

Thank you so much! 😄

Mohit Gupta added 2 commits February 12, 2020 23:16

fixes TypeError

8df27d8

solves Coercion error

7963f7c

mohitacecode requested a review from jksuom February 16, 2020 16:34

czgdp1807 added the polys label Feb 16, 2020

czgdp1807 reviewed Feb 16, 2020

View reviewed changes

improved code

f89b45f

This was referenced Feb 16, 2020

nonlinsolve raises TypeError #17276

Closed

Solves AttributeError in solveset.py #18670

Merged

jksuom reviewed Feb 17, 2020

View reviewed changes

improved docstring

b58d39a

added test

0e24f18

jksuom reviewed Feb 18, 2020

View reviewed changes

improved indentation

1f6bec2

jksuom merged commit 72e0994 into sympy:master Feb 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

solving type error and Coercion error in algebraicfield.py #18669

solving type error and Coercion error in algebraicfield.py #18669

mohitacecode commented Feb 16, 2020 •

edited by jksuom

sympy-bot commented Feb 16, 2020 •

edited

czgdp1807 Feb 16, 2020

mohitacecode Feb 16, 2020

codecov bot commented Feb 16, 2020 •

edited

mohitacecode commented Feb 17, 2020

jksuom Feb 17, 2020

mohitacecode Feb 17, 2020 •

edited

jksuom Feb 17, 2020

mohitacecode Feb 17, 2020

jksuom commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020 •

edited

mohitacecode commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

jksuom Feb 18, 2020

mohitacecode Feb 18, 2020

mohitacecode commented Feb 18, 2020

jksuom commented Feb 18, 2020

mohitacecode commented Feb 18, 2020

Iqrar99 commented Feb 23, 2020

mohitacecode commented Feb 23, 2020

Iqrar99 commented Feb 23, 2020

solving type error and Coercion error in algebraicfield.py #18669

solving type error and Coercion error in algebraicfield.py #18669

Conversation

mohitacecode commented Feb 16, 2020 • edited by jksuom

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

sympy-bot commented Feb 16, 2020 • edited

czgdp1807 Feb 16, 2020

Choose a reason for hiding this comment

mohitacecode Feb 16, 2020

Choose a reason for hiding this comment

codecov bot commented Feb 16, 2020 • edited

Codecov Report

mohitacecode commented Feb 17, 2020

jksuom Feb 17, 2020

Choose a reason for hiding this comment

mohitacecode Feb 17, 2020 • edited

Choose a reason for hiding this comment

jksuom Feb 17, 2020

Choose a reason for hiding this comment

mohitacecode Feb 17, 2020

Choose a reason for hiding this comment

jksuom commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020 • edited

mohitacecode commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

jksuom commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

mohitacecode commented Feb 17, 2020

jksuom Feb 18, 2020

Choose a reason for hiding this comment

mohitacecode Feb 18, 2020

Choose a reason for hiding this comment

mohitacecode commented Feb 18, 2020

jksuom commented Feb 18, 2020

mohitacecode commented Feb 18, 2020

Iqrar99 commented Feb 23, 2020

mohitacecode commented Feb 23, 2020

Iqrar99 commented Feb 23, 2020

mohitacecode commented Feb 16, 2020 •

edited by jksuom

sympy-bot commented Feb 16, 2020 •

edited

codecov bot commented Feb 16, 2020 •

edited

mohitacecode Feb 17, 2020 •

edited

mohitacecode commented Feb 17, 2020 •

edited