Skip to content

Conversation

@Abhinav-Khot
Copy link
Contributor

@Abhinav-Khot Abhinav-Khot commented Mar 12, 2025

Description

Modified atleast_Nd to accept only one positional argument. Also modified the tests to account for this change.

After the changes, in atleast_Nd(x, 1), x is interpreted as an array and 1 is interpreted as the dimension. This change is reflected in the tests too.

Related Issue

Checklist

Type of change

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

📚 Documentation preview 📚: https://pytensor--1291.org.readthedocs.build/en/1291/

@codecov
Copy link

codecov bot commented Mar 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.00%. Comparing base (2a7f3e1) to head (b94687f).
Report is 156 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1291   +/-   ##
=======================================
  Coverage   81.99%   82.00%           
=======================================
  Files         188      188           
  Lines       48553    48527   -26     
  Branches     8673     8667    -6     
=======================================
- Hits        39812    39793   -19     
+ Misses       6579     6578    -1     
+ Partials     2162     2156    -6     
Files with missing lines Coverage Δ
pytensor/tensor/basic.py 91.33% <100.00%> (-0.03%) ⬇️

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

def atleast_Nd(
*arys: np.ndarray | TensorVariable, n: int = 1, left: bool = True
arry: np.ndarray | TensorVariable, n: int = 1, left: bool = True
Copy link
Member

Choose a reason for hiding this comment

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

Just to avoid surprises, I would make it so you have to specify n explicitly like before. It simply doesn't accept multiple arrays anymore, so it won't be a silent change if someone was relying on this.

Later on we can allow n to be positional only like you did now.

Suggested change
arry: np.ndarray | TensorVariable, n: int = 1, left: bool = True
arry: np.ndarray | TensorVariable, *, n: int = 1, left: bool = True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made this change, modified the tests accordingly too

@ricardoV94 ricardoV94 merged commit 52bbf59 into pymc-devs:main Mar 12, 2025
73 checks passed
@ricardoV94
Copy link
Member

Thanks @Abhinav-Khot

@ricardoV94 ricardoV94 changed the title Modify atleast_Nd to accept only one positional argument Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

2 participants