Use assumption system instead of structural equality check in __bool__ method of Expression domain element

[ehren]

References to other Issues or PRs

Brief description of what is fixed or changed

Fixes #20427

Other comments

fixes an unstripped zero in result from clear_denoms that would create a Poly that prints like the zero Poly but behaves inconsistently

Release Notes

• polys
  • Use assumption system instead of structural equality check in __bool__ method of Expression domain element (fixing cases where Poly(...).is_zero would incorrectly return False)

[sympy-bot]

✅

Hi, I am the SymPy bot (v161). 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
  • Use assumption system instead of structural equality check in __bool__ method of Expression domain element (fixing cases where Poly(...).is_zero would incorrectly return False) (#20428 by @ehren)

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

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. -->


#### Brief description of what is fixed or changed

Fixes #20427 

#### Other comments

fixes an unstripped zero in result from clear_denoms that would create a Poly that prints like the zero Poly but behaves inconsistently

#### 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
    * Use assumption system instead of structural equality check in `__bool__` method of `Expression` domain element (fixing cases where `Poly(...).is_zero` would incorrectly return `False`)

<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

[codecov]

Codecov Report

Merging #20428 (22c48bd) into master (026221a) will decrease coverage by 0.005%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##            master    #20428       +/-   ##
=============================================
- Coverage   75.752%   75.747%   -0.006%     
=============================================
  Files          673       673               
  Lines       174119    174120        +1     
  Branches     41109     41108        -1     
=============================================
- Hits        131900    131892        -8     
- Misses       36505     36507        +2     
- Partials      5714      5721        +7     

[jksuom]

It seems that dup_strip should have removed EX(0) when the polynomial representation was created by dup_from_dict. It fails because the condition f[0] is True for a non-simplified element of EX:

sympy/sympy/polys/densebasic.py

Lines 265 to 276 in 026221a

	if not f or f[0]:
	return f
	i = 0
	for cf in f:
	if cf:
	break
	else:
	i += 1
	return f[i:]

The __bool__ method of EX

sympy/sympy/polys/domains/expressiondomain.py

Lines 122 to 123 in 026221a

	def __bool__(f):
	return f.ex != 0

should probably be changed to return not f.ex.is_zero.

[ehren]

should probably be changed to return not f.ex.is_zero.

That's a better fix that also removes the need for the conversion to expression in rsolve_poly in #20426

[jksuom]

Can you split this long line? It looks like ca. 10 lines would be needed. Backslashes are not necessary.

[jksuom]

Maybe a single test would suffice: assert f == Poly(0, x, domain='EX').

[ehren]

Done. That also clears up some confusion in my previous commit: Poly(...).is_zero is indeed 2 valued; Poly(y, x).is_zero is False (is not the zero poly) doesn't imply y.is_zero is False (there also shouldn't be a problem with how Poly(...).is_zero is used currently in rsolve).

[jksuom]

This looks good. Can you squash the commits?

[ehren]

squashed and reworded commit message.

[jksuom]

Thanks! Merging.

[ehren]

Thanks for the fix suggestion and review!