Skip to content
This repository has been archived by the owner on May 4, 2019. It is now read-only.

Add isless(::Nullable, ::Nullable) -> Bool #141

Merged
merged 1 commit into from
Aug 30, 2016
Merged

Add isless(::Nullable, ::Nullable) -> Bool #141

merged 1 commit into from
Aug 30, 2016

Conversation

nalimilan
Copy link
Member

This method complements isequal(), which also returns a Bool, as it
must implement a total order. NULLs are treated like NaNs: they are
never isless() than any other value, including other NULLs. They are
therefore always sorted last.

(Travis failure on master is unrelated.)

This method complements isequal(), which also returns a Bool, as it
must implement a total order. NULLs are treated like NaNs: they are
never isless() than any other value, including other NULLs. They are
therefore always sorted last.
@davidagold
Copy link
Contributor

This seems reasonable. But do you have a sense of how many more of these "special cases" (i.e., functions on Nullables that do not return Nullable) are needed?

@nalimilan
Copy link
Member Author

Very few AFAICT. Looks like we all agree that all operators should return a Nullable, except for isequal and isless, and comparison operators (==, <, >) for which there's no consensus yet. So after this PR (which I'll open against Julia Base too) it should be complete.

@davidagold
Copy link
Contributor

Yeah, I'd prefer that this live in Base. Could you please x-ref that PR when you open it?

@nalimilan
Copy link
Member Author

I'd still like this to be included in NullableArrays too so that it works on 0.4 and 0.5. Requiring 0.6 is quite cumbersome at this point.

@davidagold
Copy link
Contributor

Fair, but can we organize the files so that this method can be grouped amongst those that should really live somewhere else?

@nalimilan
Copy link
Member Author

I think that's the case. Aren't Nullable operators supposed to go into Base or into a separate package at some point? Also, this definition is very dependent on the infrastructure used for other operators.

@davidagold
Copy link
Contributor

Sounds good to me for now.

@davidagold davidagold merged commit 2e1fd1c into master Aug 30, 2016
@ararslan ararslan deleted the nl/isless branch August 30, 2016 20:09
@nalimilan
Copy link
Member Author

Filed PR against Base: JuliaLang/julia#18304

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants