Skip to content

Conversation

@LilithHafner
Copy link
Member

@LilithHafner LilithHafner added bugfix This change fixes an existing bug backport 1.11 Change should be backported to release-1.11 labels Apr 30, 2024
@KristofferC KristofferC mentioned this pull request May 6, 2024
59 tasks
@LilithHafner
Copy link
Member Author

CI is very red, but all of the failures look very unrelated (internet or file system) and master has been red for a while.

@LilithHafner LilithHafner merged commit e7a1def into master May 16, 2024
@LilithHafner LilithHafner deleted the lh/round-to-float branch May 16, 2024 11:50
@mikmoore
Copy link
Contributor

Oops. Only now did I realize that a docstring update may still be needed. I expect there are situations where an InexactError may still be thrown (or could be for some other types), so that part should still be there, but the current docstrings don't address the new behavior introduced in this PR.

@LilithHafner
Copy link
Member Author

I expect there are situations where an InexactError may still be thrown

I don't think so

julia> Base.infer_effects(round, Tuple{Type{Float64}, Int})
(+c,+e,+n,+t,+s,+m,+u)

or could be for some other types

Yes:

julia> floor(UInt8, 1000)
ERROR: InexactError: trunc(UInt8, 1000)

We also have

floor(T, x) converts the result to type T, throwing an InexactError if the floored value is not representable a T.

in the docstring already.


the current docstrings don't address the new behavior introduced in this PR.

Do you have a specific behavior that isn't documented? For the most part I think this PR is doing a bugfix to change behavior to match what was already quasi-documented.

@mikmoore
Copy link
Contributor

mikmoore commented May 16, 2024

Sorry. You're right. It's all covered. I just looked at this first thing in the morning and my brain wasn't working. It talks about rounding to representable values.

KristofferC pushed a commit that referenced this pull request May 20, 2024
- fix #52355 using option 4 (round to nearest representable integer)
- update docstrings *including documenting convert to Inf behavior even
though Inf is not the "closest" floating point value*
- add some assorted tests

---------

Co-authored-by: mikmoore <[email protected]>
(cherry picked from commit e7a1def)
KristofferC pushed a commit that referenced this pull request May 23, 2024
- fix #52355 using option 4 (round to nearest representable integer)
- update docstrings *including documenting convert to Inf behavior even
though Inf is not the "closest" floating point value*
- add some assorted tests

---------

Co-authored-by: mikmoore <[email protected]>
(cherry picked from commit e7a1def)
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label May 28, 2024
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Jul 12, 2024
…g#54314)

- fix JuliaLang#52355 using option 4 (round to nearest representable integer)
- update docstrings *including documenting convert to Inf behavior even
though Inf is not the "closest" floating point value*
- add some assorted tests

---------

Co-authored-by: mikmoore <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This change fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

round(::Type{<:AbstractFloat}, x, ::RoundingMode) violates docstring

3 participants