Skip to content

Conversation

Luis-Varona
Copy link
Contributor

Previously, the diagind documentation only covered the methods that take the full matrix as input, neglecting the methods that accept dimensions in lieu despite both styles being publicly exported. This update edits the docstring accordingly to reflect the missing methods, also adding an additional example block.

Additionally, this moves the dimension-based methods from above the matrix-based methods to below them, since the docstring is attached to the matrix-based method definition. For consistency, it further rearranges the order of the three dimension-based methods, moving the one-liners below the multi-liner just as is already the case for the matrix-based method definitions.

(Resolves #1469.)

Previously, the 'diagind' documentation only covered the methods that take the full matrix as input, neglecting the methods that accept dimensions in lieu despite both styles being publicly exported. This update edits the docstring accordingly to reflect the missing methods, also adding an additional example block.

Additionally, this moves the dimension-based methods from above the matrix-based methods to below them, since the docstring is attached to the matrix-based method definition. For consistency, it further rearranges the order of the three dimension-based methods, moving the one-liners below the multi-liner just as is already the case for the matrix-based method definitions.

(Resolves JuliaLang#1469.)
@Luis-Varona
Copy link
Contributor Author

@araujoms, here is the requested PR for #1469. Hope it helps. 🙂

Copy link

codecov bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.89%. Comparing base (28ee87e) to head (06c315d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1473   +/-   ##
=======================================
  Coverage   93.89%   93.89%           
=======================================
  Files          34       34           
  Lines       15930    15930           
=======================================
  Hits        14958    14958           
  Misses        972      972           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@araujoms
Copy link
Collaborator

Closes #1460

I think that's fine, I'll just wait a couple of days before merging to see if anybody else has comments.

@jishnub
Copy link
Member

jishnub commented Oct 14, 2025

I would suggest changing the method to diagind((m,n), k) so that, firstly, the positions of the arguments are obvious, and secondly, it's easy to pass size(A) directly to diagind. This way, diagind(A, k) would have a direct parallel in diagind(size(A), k). I understand that this is a docs PR, so we may do this separately.

@Luis-Varona
Copy link
Contributor Author

I would suggest changing the method to diagind((m,n), k) so that, firstly, the positions of the arguments are obvious, and secondly, it's easy to pass size(A) directly to diagind. This way, diagind(A, k) would have a direct parallel in diagind(size(A), k). I understand that this is a docs PR, so we may do this separately.

I think the original diagind(m, n, k) should be kept in for backwards compatibility, at the very least. We can definitely add a new diagind((m, n), k) method and maybe even mark the old one as soon-to-be-deprecated, but in the next PR we probably shouldn't completely replace the old method.

@araujoms araujoms merged commit 52c41f7 into JuliaLang:master Oct 15, 2025
4 checks passed
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.

Missing methods in diagind documentation

3 participants