Skip to content

Modify atleast_Nd to accept only one positional argument #1291

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 3 commits into from
Mar 12, 2025

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/

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 15 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.

@@ -4355,28 +4355,22 @@ def empty_like(


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 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

atleast_Nd should only accept one positional argument
2 participants