Skip to content

Rename sgn to sign #228

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.

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

Merged
merged 4 commits into from
Mar 6, 2023
Merged

Rename sgn to sign #228

merged 4 commits into from
Mar 6, 2023

Conversation

sudarsan2k5
Copy link
Contributor

fixes #162

  • Rename sgn with sign
  • Deprecated sgn

Checklist

@twiecki
Copy link
Member

twiecki commented Feb 26, 2023

@sudarsan2k5 Thanks! Note the conflicts, you need to rebase from main.

@sudarsan2k5 sudarsan2k5 marked this pull request as draft February 26, 2023 18:22
@sudarsan2k5 sudarsan2k5 force-pushed the rename_sgn branch 2 times, most recently from 45549f6 to 41ce74d Compare February 26, 2023 19:21
@sudarsan2k5 sudarsan2k5 marked this pull request as ready for review February 26, 2023 19:24
@sudarsan2k5
Copy link
Contributor Author

by mistakenly converted to draft

@codecov-commenter
Copy link

Codecov Report

Merging #228 (0218c36) into main (63f8d6e) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #228   +/-   ##
=======================================
  Coverage   80.43%   80.43%           
=======================================
  Files         170      170           
  Lines       45326    45326           
  Branches     9687     9687           
=======================================
  Hits        36457    36457           
  Misses       6643     6643           
  Partials     2226     2226           
Impacted Files Coverage Δ
pytensor/compile/profiling.py 75.35% <ø> (ø)
pytensor/scalar/basic.py 79.77% <100.00%> (ø)
pytensor/sparse/basic.py 82.49% <100.00%> (ø)
pytensor/tensor/inplace.py 100.00% <100.00%> (ø)
pytensor/tensor/math.py 90.69% <100.00%> (ø)
pytensor/tensor/rewriting/math.py 86.03% <100.00%> (ø)
pytensor/tensor/subtensor.py 89.64% <100.00%> (ø)

@@ -1092,7 +1092,7 @@ def log1p(a):


@scalar_elemwise
def sgn(a):
def sign(a):
"""sign of a"""


Copy link
Member

Choose a reason for hiding this comment

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

Do you mind adding a compatible sgn like this (and make it accessible from pytensor.tensor as before.

Suggested change
def sgn(a):
warnings.warn(
"sgn is deprecated and will stop working in the future, use sign instead.",
FutureWarning,
)
return sign(a)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ricardoV94 I have implemented the suggested changes, could you please check if we need anything else in this PR?

@ricardoV94
Copy link
Member

@sudarsan2k5 Sorry I just wanted to check locally the old import is still working. I'll do it tomorrow and merge after

@ricardoV94 ricardoV94 merged commit 0b632bd into pymc-devs:main Mar 6, 2023
@ricardoV94
Copy link
Member

Thanks @sudarsan2k5, merged!

@ricardoV94 ricardoV94 changed the title Rename sgn with sign Rename sgn to sign Mar 6, 2023
ricardoV94 pushed a commit to ricardoV94/pytensor that referenced this pull request Mar 6, 2023
* Rename sgn to sign
ricardoV94 pushed a commit to ricardoV94/pytensor that referenced this pull request Mar 6, 2023
* Rename sgn to sign
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement numpy.sign equivalent
4 participants