Fix Scalar and Endpoint equality #56
Conversation
- Added == functions for (left,left), (right,right), (scalar,left), (scalar, right), (left, scalar), (right, scalar) - Added isequal functions for (left,left) and (right,right) - Changed some functions to not be repetitions of previous code such as Left == Right code will just call Right == Left instead of redefining the same code within both functions. - Added First = Left, and Last = Right shortcuts. - Added some more Scalar tests because why not
src/endpoint.jl
Outdated
| function Base.:(==)(a::RightEndpoint, b::RightEndpoint) | ||
| a.endpoint == b.endpoint && a.included == b.included | ||
| end | ||
|
|
There was a problem hiding this comment.
Both of these are covered by falling back to Base.:(==)(a::Endpoint, b::Endpoint)
There was a problem hiding this comment.
Thats what I wanted to assume, but I was getting these weird issues where it seemed like it wasnt. Perhaps I was somewhat misunderstanding what I was doing. Tests definitely seem to run fine without them as they are falling back as you mentioned.
src/endpoint.jl
Outdated
| end | ||
|
|
||
| function Base.:(==)(a::RightEndpoint, b) | ||
| b == a | ||
| end |
There was a problem hiding this comment.
Can just use Endpoint here:
function Base.:(==)(a, b::Endpoint)
a == b.endpoint && b.included
end
function Base.:(==)(a::Endpoint, b)
b == a
endI assume the one-liner versions of these look out of place?
There was a problem hiding this comment.
These should probably be moved to the end with the isless(::Any, ::Endpoint) definitions. Since the logic behind scalar-endpoint and endpoint-endpoint comparisons is rather different I think it makes sense to group by this way instead of by operator.
There was a problem hiding this comment.
Right, that makes sense. I suppose some of these could be one liners, but then the question is if we want it to be consistently long versions for the entire thing, or mixed, or make them all one liners. Some are little long though
src/endpoint.jl
Outdated
|
|
||
| function Base.isequal(a::RightEndpoint, b::RightEndpoint) | ||
| isequal(a.endpoint, b.endpoint) && isequal(a.included, b.included) | ||
| end |
src/endpoint.jl
Outdated
| @@ -6,6 +6,8 @@ const Right = Direction{:Right}() | |||
| const Beginning = Left | |||
| const Ending = Right | |||
|
|
|||
| const First = Left | |||
| const Last = Right | |||
There was a problem hiding this comment.
I would save this for another PR.
Codecov Report
@@ Coverage Diff @@
## master #56 +/- ##
==========================================
+ Coverage 98.26% 98.27% +0.01%
==========================================
Files 6 6
Lines 230 232 +2
==========================================
+ Hits 226 228 +2
Misses 4 4
Continue to review full report at Codecov.
|
- Potentially had some weird environment thing where it seemed like some comparisons weren't falling back to their Endpoint versions. Possibly I was doing something weird as the tests all work fine without those extra comparison functions I made. - Moved all the scalar comparison functions to the bottom of the file, and made them all one liners as they are all relatively small. - Removed the First/Last thing as it isn't really needed for this PR.
This fixes the "==" issue in #54
Also adds the Left/Right consts.
(scalar, right), (left, scalar), (right, scalar)
Left == Right code will just call Right == Left instead of redefining
the same code within both functions.