Create min/max functions that take bounds into account #141
Conversation
Arvind-Maan
force-pushed
the
am/bounded-first/last
branch
from
cfe1c7b
to
4f760f6
Compare
Arvind-Maan
changed the title
Codecov Report
@@ Coverage Diff @@
## master #141 +/- ##
==========================================
+ Coverage 74.40% 75.76% +1.36%
==========================================
Files 11 11
Lines 461 491 +30
==========================================
+ Hits 343 372 +29
- Misses 118 119 +1
Continue to review full report at Codecov.
|
Arvind-Maan
force-pushed
the
am/bounded-first/last
branch
from
736521c
to
3dc8a8b
Compare
|
Another thing I'm thinking about is the name "precision". Currently Julia already has a |
|
As a follow-up to this PR, it'd be nice to resolve #90 😊 |
Arvind-Maan
force-pushed
the
am/bounded-first/last
branch
from
2342100
to
6d019b4
Compare
Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>
Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>
Arvind-Maan
force-pushed
the
am/bounded-first/last
branch
from
6d019b4
to
3d60fbb
Compare
|
Thanks for sticking with this. Things are shaping up nicely now :) |
Co-Authored-By: Curtis Vogt <1675958+omus@users.noreply.github.com>
Co-Authored-By: Curtis Vogt <1675958+omus@users.noreply.github.com>
|
|
||
| # If we get to this point, the min/max functions throw a DomainError | ||
| # Since we want our tests to be predictable, we will not throw an error in this helper. | ||
| end |
There was a problem hiding this comment.
As theses helpers pretty much just re-implement minimum/maximum there aren't that helpful with testing. I think we can proceed with this but something to keep in mind for future work.
There was a problem hiding this comment.
I agree, in hindsight it would've been best to separate the tests per edge case and avoid having to do this. I constricted myself by trying to stay consistent with the first/last testing scheme and the test_values that were already in place.
src/docstrings.jl
Outdated
| minimum(interval::AbstractInterval{T}; [increment]) -> T | ||
|
|
||
| The minimum value in the interval contained within the interval. |
There was a problem hiding this comment.
| minimum(interval::AbstractInterval{T}; [increment]) -> T | |
| The minimum value in the interval contained within the interval. | |
| minimum(interval::AbstractInterval{T}; [increment]) -> T | |
| The minimum value contained within the `interval`. |
There was a problem hiding this comment.
Looks like this change was missed?
src/docstrings.jl
Outdated
| maximum(interval::AbstractInterval{T}; increment::Any) -> T | ||
|
|
||
| The maximum value in the interval contained within the interval. |
There was a problem hiding this comment.
| maximum(interval::AbstractInterval{T}; increment::Any) -> T | |
| The maximum value in the interval contained within the interval. | |
| maximum(interval::AbstractInterval{T}; [increment]) -> T | |
| The maximum value contained within the `interval`. |
There was a problem hiding this comment.
Looks like this change was missed?
* Add min/max functions with precision * Add tests for new min/max functions * Add default val for closed/unbounded intervals in min/max * Rename min/max functions to minimum/maximum * Rename percision kwarg in min/max to increment * Add integer specific min/max function * Add float specific min/max functions * Respect the increment variable in AbstractFloat min/max * Apply suggestions and add more tests * Add unbounded accessor tests * Add anchoredinterval support in min/max * Adjust accessor test structure * Handle various edge cases in min/max * Add docstrings for min/max * Update isfinite.jl to support TimeTypes * Apply suggested changes * Update docs to include bound-open Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>
|
PR was meant to be squashed. Squashed commit is c37cbb7 |
fixes #137