Skip to content

Conversation

ricardoV94
Copy link
Member

This showed up in https://discourse.pymc.io/t/m-h-algorithm-with-custom-distribution-with-scipy/11866/4?u=ricardov94

I don't see any reason to break away from Python indexing expectations...

@codecov-commenter
Copy link

Codecov Report

Merging #274 (5071a7d) into main (91046b6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #274   +/-   ##
=======================================
  Coverage   80.46%   80.46%           
=======================================
  Files         170      170           
  Lines       45412    45408    -4     
  Branches    11087    11085    -2     
=======================================
- Hits        36541    36539    -2     
+ Misses       6639     6638    -1     
+ Partials     2232     2231    -1     
Impacted Files Coverage Δ
pytensor/tensor/basic.py 90.62% <ø> (+0.13%) ⬆️
pytensor/graph/basic.py 88.84% <100.00%> (-0.07%) ⬇️
pytensor/tensor/subtensor.py 89.64% <100.00%> (ø)

@ricardoV94 ricardoV94 requested a review from twiecki April 12, 2023 17:30
Comment on lines -198 to -200
raise ValueError(f"{self.op}.default_output should be an int or long")
elif do < 0 or do >= len(self.outputs):
raise ValueError(f"{self.op}.default_output is out of range.")
Copy link
Member Author

Choose a reason for hiding this comment

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

I think Python error messages do a good enough job here when indexing is not valid or out of bounds. No need to reinvent the wheel.

@michaelosthege michaelosthege merged commit acfa749 into pymc-devs:main Apr 13, 2023
@ricardoV94 ricardoV94 added the bug Something isn't working label Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants