Skip to content

fixed max(), min(), range() for multiple inputs if one is non-finite#181

Merged
MichaelChirico merged 29 commits intor-lib:mainfrom
hcirellu:main
Nov 7, 2025
Merged

fixed max(), min(), range() for multiple inputs if one is non-finite#181
MichaelChirico merged 29 commits intor-lib:mainfrom
hcirellu:main

Conversation

@hcirellu
Copy link
Copy Markdown
Collaborator

For #142
The example in issue #142 now works correctly.
I also fixed the behaviour that the warning is also shown in the case of range(NA_integer64_, na.rm=TRUE).
The same is applied to min.integer64 and max.integer64.

@MichaelChirico
Copy link
Copy Markdown
Collaborator

Thanks! Please add regresion tests

Comment thread R/integer64.R Outdated
@hcirellu
Copy link
Copy Markdown
Collaborator Author

Regression tests added and reverted to =.

@hcirellu
Copy link
Copy Markdown
Collaborator Author

Is there anything more to do or can it be merged?

Comment thread R/integer64.R
@MichaelChirico
Copy link
Copy Markdown
Collaborator

Is there anything more to do or can it be merged?

I just need to find some time, hopefully this weekend.

Comment thread R/integer64.R Outdated
Comment thread NEWS.md Outdated
Comment thread tests/testthat/test-bit64-package.R
Comment thread R/integer64.R Outdated
Comment thread R/integer64.R Outdated
Comment thread R/integer64.R Outdated
Comment thread R/integer64.R Outdated
Copy link
Copy Markdown
Collaborator

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

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

Sorry for the extra round of review. I tried making these edits directly myself but because your PR is from your main branch, I am unable to do so (or otherwise, you didn't grant me access to edit your fork)

@hcirellu
Copy link
Copy Markdown
Collaborator Author

hcirellu commented Oct 30, 2025

Sorry for the extra round of review. I tried making these edits directly myself but because your PR is from your main branch, I am unable to do so (or otherwise, you didn't grant me access to edit your fork)

I thought I allowed it. I'll check.
I added you as collaborator. I hope that helps.

Copy link
Copy Markdown
Collaborator

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

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

OK I think this is ready to go. Thanks!!

@MichaelChirico MichaelChirico merged commit e08a184 into r-lib:main Nov 7, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants