8000 ENH: correct identity for logaddexp2 ufunc: -inf by eriknw · Pull Request #16102 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

ENH: correct identity for logaddexp2 ufunc: -inf #16102

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
Merged
merged 4 commits into from
May 13, 2020
Merged

ENH: correct identity for logaddexp2 ufunc: -inf #16102

merged 4 commits into from
May 13, 2020

Conversation

eriknw
Copy link
Contributor
@eriknw eriknw commented Apr 28, 2020

The lack of identity for logaddexp2 was first identified in #4599. The implementation in #8955 added -inf as identity for logaddexp, but missed adding it for logaddexp2.

(This is my first PR to numpy!)

@eriknw
ENH: correct identity for logaddexp2 ufunc: -inf
792480b 
The lack of identity for `logaddexp2` was first identitifed in numpy#4599.
The implementation in numpy#8955 added -inf as identity for `logaddexp`,
but missed adding it for `logaddexp2`.
@charris
Copy link
Member
charris commented Apr 29, 2020

Error looks unrelated. I believe @mattip is working on a fix.

eric-wieser
eric-wieser reviewed Apr 29, 2020
View reviewed changes
@eriknw @eric-wieser
ENH: test that -inf is identity of logaddexp2
3a72658 
Co-Authored-By: Eric Wieser <wieser.eric@gmail.com>
@eric-wieser
Copy link
Member

CI failure is bizarre:

  File "c:\hostedtoolcache\windows\python\3.6.8\x64\lib\distutils\command\build_ext.py", line 473, in _build_extensions_serial
      self.build_extension(ext)
    File "setup.py", line 260, in build_extension
      build_ext.build_extension(self, ext)
    File "D:\a\1\s\numpy\distutils\command\build_ext.py", line 478, in build_extension
      unlinkable_fobjects)
    File "D:\a\1\s\numpy\distutils\command\build_ext.py", line 523, in _process_unlinkable_fobjects
      fobjects = [os.path.relpath(obj) for obj in unlinkabl
8000
e_fobjects]
    File "D:\a\1\s\numpy\distutils\command\build_ext.py", line 523, in <listcomp>
      fobjects = [os.path.relpath(obj) for obj in unlinkable_fobjects]
    File "c:\hostedtoolcache\windows\python\3.6.8\x64\lib\ntpath.py", line 584, in relpath
      path_drive, start_drive))
  ValueError: path is on mount 'c:', start on mount 'D:'

Looks like a real bug in numpy (not being installable across drive boundaries), but clearly not the fault of this PR.

@mattip
Copy link
Member
mattip commented Apr 29, 2020

Azure changed something. PR gh-16101 is waiting for review

@mattip
Copy link
Member
mattip commented Apr 29, 2020

Note this is pretty critical since the macos documentation checks are all run only if a preliminary windows run succeeds, so nep and doc PRs should not be merged till this is fixed

@seberg
Copy link
Member
seberg commented Apr 29, 2020

One slight curiosity is that np.log(0) should give a warning, because an inf is created. So arguable np.logaddexp.reduce([]) should also. We do not have that capability, and its not vital, just thought I would mention. This seems simple and good to go, unless anyone thinks it should have a release note?

@seberg seberg closed this Apr 30, 2020
@seberg seberg reopened this Apr 30, 2020
@seberg
Copy link
Member
seberg commented Apr 30, 2020

Thanks for a nice first PR, looking forward to more! (Just waiting for the test to finish before merging).

@seberg
Copy link
Member
seberg commented Apr 30, 2020

Oh, I forgot about the potential inclusion in the release notes. @eriknw can you add a very short release note statement, as detailed in https://github.com/numpy/numpy/blob/master/doc/release/upcoming_changes/README.rst

@rossbar rossbar added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label May 8, 2020
@eriknw
Copy link
Contributor Author
eriknw commented May 13, 2020

Release note added. Sorry for the delay.

seberg
seberg reviewed May 13, 2020
View reviewed changes
@eriknw @seberg
Update doc/release/upcoming_changes/16102.improvement.rst
21ad7ca 
ENH: use valid link in update note

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
@eric-wieser eric-wieser removed the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label May 13, 2020
@seberg seberg merged commit 5f7114c into numpy:master May 13, 2020
@seberg
Copy link
Member
seberg commented May 13, 2020

Thanks for the followup!

@eriknw
Copy link
Contributor Author
eriknw commented May 13, 2020

My pleasure, and thanks for being very welcoming, prompt, and helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seberg seberg seberg left review comments

@eric-wieser eric-wieser eric-wieser left review comments

Assignees
No one assigned
Labels
01 - Enhancement component: numpy.ufunc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@eriknw @charris @eric-wieser @mattip @seberg @rossbar
0