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.

Sign up for GitHub

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

Jump to bottom

Generic isfinite method #36380

Merged
merged 4 commits into from
Sep 18, 2020
Merged

Generic isfinite method #36380

merged 4 commits into from
Sep 18, 2020

Conversation

omus
Copy link
Member

@omus omus commented Jun 22, 2020

Implements a generic isfinite method which should work for any type supporting subtraction and iszero. My motivation behind the change is that I was writing some generic code which needed isfinite to work with Char and Period instances.

@omus omus added maths Mathematical functions dates Dates, times, and the Dates stdlib module labels Jun 22, 2020
omus
omus commented Jun 22, 2020
View reviewed changes
base/float.jl Show resolved Hide resolved
@omus omus mentioned this pull request Jun 23, 2020
Merged
@omus
Copy link
Member Author

omus commented Sep 8, 2020

Rebased and resolved conflict

@omus omus added the triage This should be discussed on a triage call label Sep 9, 2020
@omus
Copy link
Member Author

omus commented Sep 10, 2020

From triage. Seems better to narrow the scope to isfinite(::Number) and also define isfinite(::Period). As Julia has moved away from treating characters as numbers we should leave isfinite(::Char) as undefined.

@omus omus removed the triage This should be discussed on a triage call label Sep 10, 2020
@omus omus mentioned this pull request Sep 14, 2020
Merged
omus
omus commented Sep 14, 2020
View reviewed changes
stdlib/Dates/src/periods.jl
@@ -54,6 +54,7 @@ Base.zero(::Union{Type{P},P}) where {P<:Period} = P(0)
Base.one(::Union{Type{P},P}) where {P<:Period} = 1 # see #16116
Base.typemin(::Type{P}) where {P<:Period} = P(typemin(Int64))
Base.typemax(::Type{P}) where {P<:Period} = P(typemax(Int64))
Base.isfinite(::Union{Type{P}, P}) where {P<:Period} = true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the lead of how isfinite is implemented for TimeType as that also supports taking the type

@omus
Copy link
Member Author

omus commented Sep 18, 2020

The usefulness of isfinite(::Number) is definitely debatable as currently all numeric types in base implement a more specific version. I was thinking about removing it from this PR but it should still be useful for custom Number types as well as providing a template for extending this to other types in the future.

@omus omus merged commit 109a1e1 into master Sep 18, 2020
@omus omus deleted the cv/generic-isfinite branch September 18, 2020 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
dates Dates, times, and the Dates stdlib module maths Mathematical functions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@omus