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

Tighten signature of iszero and isone fallbacks #57671

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lgoettgens
Copy link
Contributor

Resolves #57372. Please see that issue for the motivation behind the change.

Could someone please run a PkgEval on this? If there are breakages in the ecosystem, I think this is a sign to not follow down this any further.

@nsajko nsajko added needs pkgeval Tests for all registered packages should be run with this change maths Mathematical functions DO NOT MERGE Do not merge this PR! labels Mar 7, 2025
@nsajko
Copy link
Contributor

nsajko commented Mar 7, 2025

This PR currently breaks at least the following functionality:

  • linear algebra
  • dates
  • TwicePrecision (used for floating-point ranges)

@jishnub
Copy link
Member

jishnub commented Mar 8, 2025

This seems to be quite breaking for somewhat limited gains. A lot of the breakage might happen in the wild, and wouldn't be caught by a PkgEvalrun.

I'm also not fully certain about the motivation here. One of the examples in the issue is iszero(Int) returns false, but probably a specific method should be added for Types to avoid such comparisons? Also, probably a similar method may be added for Ring? After all, iszero(x) is documented to be a check for x == zero(x), where zero(x) is the additive identity. Special cases should generally be added when this isn't what zero returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Do not merge this PR! maths Mathematical functions needs pkgeval Tests for all registered packages should be run with this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tighten signature of iszero and isone fallbacks?
3 participants