Skip to content

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented May 29, 2025

@AllenDowney I cherry-picked your changes from #1427, I squashed all the transpose related changes into a single one, and split unrelated changes into other 2 commits.

I pushed a fourth commit with changes that I'm listing here, and I would appreciate if you could in turn review them:

  1. Moved the missing_dims out of the Op since this is not really a parameter that affects how the Op behaves at runtime, and is therefore useless. I put the behavior in the helper that calls the Transpose Op instead. The Op will expect dims to be correctly specified and raise if not.
  2. Merged the helper expand_ellipsis into the make_node since that's the only place it's used and it's not insanely complex.
  3. Put the 2 new transpose tests closer to the original one, and simplified the last test, no point (imo) in testing 2 functions for x.T
  4. Tweaked error messages to match what xarray returns.

@AllenDowney
Copy link
Contributor

@ricardoV94 Your changes look good to me (and Cursor).

@ricardoV94
Copy link
Member Author

@ricardoV94 Your changes look good to me (and Cursor).

AIs are not very adversarial by default, but good to know. All merge it then

@ricardoV94 ricardoV94 merged commit 3f5194f into pymc-devs:labeled_tensors May 29, 2025
2 of 4 checks passed
@ricardoV94 ricardoV94 deleted the transpose branch May 29, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants