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.

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

Update tensor.where to allow for case with only condition #844

Merged
merged 8 commits into from
Jun 24, 2024

Conversation

tanish1729
Copy link
Contributor

@tanish1729 tanish1729 commented Jun 22, 2024

Description

Allows tensor.where to work similar to np.where in case only a condition is given

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

Copy link

codecov bot commented Jun 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.89%. Comparing base (d3bd1f1) to head (49153cd).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #844   +/-   ##
=======================================
  Coverage   80.89%   80.89%           
=======================================
  Files         169      169           
  Lines       46979    46984    +5     
  Branches    11478    11480    +2     
=======================================
+ Hits        38002    38007    +5     
- Misses       6764     6767    +3     
+ Partials     2213     2210    -3     
Files Coverage Δ
pytensor/tensor/basic.py 88.47% <100.00%> (+0.03%) ⬆️

... and 3 files with indirect coverage changes

@tanish1729 tanish1729 marked this pull request as draft June 22, 2024 12:33
@tanish1729 tanish1729 marked this pull request as ready for review June 22, 2024 12:34
pytensor/tensor/basic.py Outdated Show resolved Hide resolved
pytensor/tensor/basic.py Outdated Show resolved Hide resolved
@ricardoV94
Copy link
Member

Needs a test

@tanish1729
Copy link
Contributor Author

should i also be testing for cases where ift and iff are being passed or only for the new case which we deal with?

@ricardoV94
Copy link
Member

should i also be testing for cases where ift and iff are being passed or only for the new case which we deal with?

Yeah that makes sense, and also test the error case

@tanish1729
Copy link
Contributor Author

in the case where it should raise an exception, it is correctly doing that but the np.testing.assert_allclose wont pass in that case. how to check for cases where an error should be raised?

@ricardoV94
Copy link
Member

ricardoV94 commented Jun 23, 2024

in the case where it should raise an exception, it is correctly doing that but the np.testing.assert_allclose wont pass in that case. how to check for cases where an error should be raised?

with pytest.raises context manager

https://docs.pytest.org/en/7.1.x/how-to/assert.html

@tanish1729
Copy link
Contributor Author

everything seems to be working fine now. lmk if any more changes are required or if some implementation should be done differently

pytensor/tensor/basic.py Outdated Show resolved Hide resolved
tests/tensor/test_basic.py Outdated Show resolved Hide resolved
@ricardoV94 ricardoV94 changed the title Updated tensor.where to allow for case with only condition Update tensor.where to allow for case with only condition Jun 24, 2024
@ricardoV94 ricardoV94 merged commit 7159215 into pymc-devs:main Jun 24, 2024
57 checks passed
@ricardoV94
Copy link
Member

Thanks @tanish1729

@tanish1729 tanish1729 deleted the update-pt-where branch July 3, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tensor.where does not behave like np.where when only one input (condition) is passed
3 participants