Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Export nonmissingtype #31562

Closed
wants to merge 1 commit into from
Closed

Export nonmissingtype #31562

wants to merge 1 commit into from

Conversation

juliohm
Copy link
Contributor

@juliohm juliohm commented Mar 30, 2019

Fix #30657

@stevengj stevengj added needs docs Documentation for this change is required missing data Base.missing and related functionality needs news A NEWS entry is required for this change labels Mar 30, 2019
@juliohm
Copy link
Contributor Author

juliohm commented Apr 4, 2019

I don't have time at the moment to work on this. Please feel free to improve the PR.

@nalimilan nalimilan mentioned this pull request Jun 14, 2019
Open
@juliohm
Copy link
Contributor Author

juliohm commented Jul 27, 2019

Can this be merged into Julia 1.2 or Julia 1.3?

@StefanKarpinski StefanKarpinski added the triage This should be discussed on a triage call label Jul 27, 2019
@vtjnash
Copy link
Member

vtjnash commented Aug 8, 2019

Perhaps should wait for #31602?

@StefanKarpinski
Copy link
Member

Too late for 1.2, can go into 1.3. As long as #31602 also goes into 1.3, this is fine 👍

@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Aug 8, 2019
@StefanKarpinski StefanKarpinski added this to the 1.3 milestone Aug 8, 2019
@JeffBezanson
Copy link
Member

From #31602, there is a question on whether this should check to make sure it could do anything; e.g. nonmissingtype(Any) == Any, which might violate your assumptions.

Mostly this needs docs though.

@StefanKarpinski
Copy link
Member

What about calling this subtract_missing to match Base.subtract_singletontype?

@JeffBezanson
Copy link
Member

Thank goodness subtract_singletontype isn't exported :) That is a pretty rancid name/concept.

@StefanKarpinski
Copy link
Member

I find subtractmissing to be a clearer name for what this does than nonmissingtype which seems like it might be an assertion or something? I would have no immediate guess about what a function by that name would do. The name subtractmissing at least gives me a hint.

@juliohm
Copy link
Contributor Author

juliohm commented Aug 10, 2019

I personally find subtractmissing confusing. It may read well for core developers, but end users will ask why "subtraction"? I'd even shorten the name further and write nonmissing. The "type" suffix is intuitive from the call? nonmissing(T)

@JeffBezanson
Copy link
Member

Please add docs and news, then triage is ok with the name nonmissingtype.

@juliohm
Copy link
Contributor Author

juliohm commented Aug 15, 2019 via email

@JeffBezanson
Copy link
Member

Docs added and merged.

JeffBezanson pushed a commit that referenced this pull request Aug 15, 2019
@juliohm @JeffBezanson
Export nonmissingtype (#31562)
1e6d5f1 
Fix #30657
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
missing data Base.missing and related functionality needs docs Documentation for this change is required needs news A NEWS entry is required for this change
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.

Export Base.nonmissingtype
5 participants
@juliohm @vtjnash @StefanKarpinski @JeffBezanson @stevengj