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

Add inv(::Missing) #31451

Merged
merged 2 commits into from
Mar 26, 2019
Merged

Add inv(::Missing) #31451

merged 2 commits into from
Mar 26, 2019

Conversation

narendrakpatel
Copy link
Contributor

Closes #31408

Adds inv(::Missing)
Adds test for inv(::Missing)

@ararslan ararslan added needs news A NEWS entry is required for this change missing data Base.missing and related functionality needs compat annotation Add !!! compat "Julia x.y" to the docstring labels Mar 22, 2019
nalimilan
nalimilan reviewed Mar 22, 2019
View reviewed changes
test/missing.jl Outdated
@@ -81,7 +81,7 @@ end
arithmetic_operators = [+, -, *, /, ^, Base.div, Base.mod, Base.fld, Base.rem]

# All unary operators return missing when evaluating missing
for f in [!, ~, +, -]
for f in [!, ~, +, -, inv]
Copy link
Member

Choose a reason for hiding this comment

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

Can you put with with elementary functions instead?

Also, please add an entry in NEWS.md.

Copy link
Contributor Author

@narendrakpatel narendrakpatel Mar 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

About the compat annotation, Should I just add inv under unary operators in missing.md?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I'm not sure what you mean. In NEWS.md, please add an entry under "New language features".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check the most recent commit. My query was regarding needs compat annotation label.

Copy link
Member

@nalimilan nalimilan Mar 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

My query was regarding needs compat annotation label.

Ah, OK. I don't think it's appropriate since there's no docstring. @ararslan?

EDIT: I guess we could add a !!! compat "Julia 1.2" block to the inv docstring mentioning missing values.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it should be in the existing docstring for inv

@ararslan ararslan removed needs compat annotation Add !!! compat "Julia x.y" to the docstring needs news A NEWS entry is required for this change labels Mar 25, 2019
@narendrakpatel
Copy link
Contributor Author

If this is okay, can this be merged? @ararslan

@ararslan ararslan merged commit 8649af9 into JuliaLang:master Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ararslan ararslan ararslan approved these changes

@nalimilan nalimilan nalimilan approved these changes

Assignees
No one assigned
Labels
missing data Base.missing and related functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@narendrakpatel @nalimilan @ararslan