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

Deprecate conj(::Any), add real(::Missing) and imag(::Missing) #26288

Merged
merged 4 commits into from
Mar 2, 2018
Merged

Deprecate conj(::Any), add real(::Missing) and imag(::Missing) #26288

merged 4 commits into from
Mar 2, 2018

Conversation

dlfivefifty
Copy link
Contributor

This resolves issue #26248

Sacha0
Sacha0 reviewed Mar 1, 2018
View reviewed changes
Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

Thanks @dlfivefifty! :) Out of curiosity, why the associated real and imag changes? Also, perhaps a brief news entry is in order?

@dlfivefifty
Copy link
Contributor Author

I noticed that real(::Missing) and imag(::Missing) weren't implemented (threw an error) and so I added them.

Unless they were left off intentionally.

nalimilan
nalimilan approved these changes Mar 1, 2018
View reviewed changes
Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

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

Thanks! Could you make the same changes in Missings.jl for 0.6 compatibility?

@JeffBezanson
Copy link
Member

Also reminds me we need to finish #25502

@quinnj
Copy link
Member

quinnj commented Mar 1, 2018

@dlfivefifty, any interest in just adding #25502 (comment) into this PR? These changes are so small...

@dlfivefifty
Copy link
Contributor Author

any interest in just adding #25502 (comment)

Isn't that PR fine as is?

@fredrikekre
Copy link
Member

At this rate of adding special missing methods I feel like we should instead have something like (::Function)(::Missing) = missing instead of go digging for methods that dont have their missing method defined.

@nalimilan
Copy link
Member

At this rate of adding special missing methods I feel like we should instead have something like (::Function)(::Missing) = missing instead of go digging for methods that dont have their missing method defined.

That's been evoked at least since Nullable was introduced. Indeed that would make a lot of sense for one-argument methods, but then it's more difficult when there are several arguments as we have no way currently to express "at least one argument is missing". We would also need a special mechanism to prevent ambiguities from arising everywhere. In theory this could be introduced in 1.x without breaking code, at least if methods with arguments marked as ::Any take precedence over the missing fallback.

There's also the objection that missing generally only makes sense where a scalar would be passed. It's probably better to have det(missing) or permutedims(missing) throw an error than return missing (though in practice you'll likely get an error soon enough anyway).

@dlfivefifty
Copy link
Contributor Author

I think a less dangerous approach would be to keep an official list of "math" functions a la Calculus.symbolic_derivatives_1arg that can be looped over.

@JeffBezanson
Copy link
Member

I'll remind everybody here of the terms of the missing deal: I was assured that those who wanted missing would content themselves with a fixed set of commonly-used functions (as in e.g. C#), and use some kind of manual lifting for everything else.

@JeffBezanson JeffBezanson merged commit ade40f6 into JuliaLang:master Mar 2, 2018
@JeffBezanson JeffBezanson mentioned this pull request Mar 2, 2018
Closed
mbauman added a commit that referenced this pull request Mar 5, 2018
@mbauman
Merge remote-tracking branch 'origin/master' into mb/deprecatemultiva…
95a78df 
…luenonscalarindexedassignment

* origin/master: (28 commits)
  fix an optimizer bug in `invoke` with non-constant functions (#26301)
  lower top-level statements in such a way that the front-end knows (#26304)
  Make sure Sockets page has h1 header (#26315)
  fix doctests, and make them less prone to errors (#26275)
  FIx intro to manual chapter on types (#26312)
  Add a missing "that" (#26313)
  fix docstring for code_llvm (#26266)
  Remove the examples/ folder (#26153)
  download cert.pem from deps-getall, fixes #24903 (#25344)
  Slight update to doc string for at-enum to refer to instances (#26208)
  performance tweak in reverse(::String) (#26300)
  remove references to `TCPSocket` in Base docstrings (#26298)
  Deprecate adding integers to CartesianIndex (#26284)
  Deprecate conj(::Any), add real(::Missing) and imag(::Missing) (#26288)
  fix #26267, regression in `names` with `imported=false` (#26293)
  fix #25857, `typeinfo` problem in showing arrays with abstract tuple types (#26289)
  Add adjoint(::Missing) method (#25502)
  Use lowered form in logging macro (#26291)
  deprecate bin, oct, dec, hex, and base in favor of `string` and keyword args (#25804)
  deprecate `spawn(cmd)` to `run(cmd, wait=false)` (#26130)
  ...
@david-pl david-pl mentioned this pull request May 29, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sacha0 Sacha0 Sacha0 left review comments

@nalimilan nalimilan nalimilan approved these changes

@andreasnoack andreasnoack andreasnoack approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@dlfivefifty @JeffBezanson @quinnj @fredrikekre @nalimilan @andreasnoack @Sacha0