Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix stuff with diagonals #1044

Merged
merged 10 commits into from May 28, 2022
Merged

Fix stuff with diagonals #1044

merged 10 commits into from May 28, 2022

Conversation

Steffan153
Copy link
Contributor

Fixes #1042
Fixes #1043

@Vyxal-Bot-old Vyxal-Bot-old added the PR: Bug Fix PR only tag - a PR that is fixing a bug/issue label May 28, 2022
@Steffan153 Steffan153 added PR: Element Implementation PR only tag - a PR that is implementing an element request. PR: Enhancement PR only tag - for enhancements labels May 28, 2022
cgccuser
cgccuser approved these changes May 28, 2022
View reviewed changes
Copy link
Member

@cgccuser cgccuser left a comment

Choose a reason for hiding this comment

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

LGTM, just some suggestions to check for empty lists

vyxal/elements.py Show resolved Hide resolved
vyxal/elements.py Show resolved Hide resolved
vyxal/elements.py Show resolved Hide resolved
cgccuser
cgccuser requested changes May 28, 2022
View reviewed changes
Copy link
Member

@cgccuser cgccuser left a comment

Choose a reason for hiding this comment

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

Grr

Steffan153 and others added 3 commits May 28, 2022 13:09
@Steffan153 @cgccuser
Update vyxal/elements.py
23d6e5a 
Co-authored-by: cgccuser <91574512+cgccuser@users.noreply.github.com>
@Steffan153 @cgccuser
Update vyxal/elements.py
d5b1d7c 
Co-authored-by: cgccuser <91574512+cgccuser@users.noreply.github.com>
@Steffan153 @cgccuser
Update vyxal/elements.py
8318c0e 
Co-authored-by: cgccuser <91574512+cgccuser@users.noreply.github.com>
@cgccuser
Copy link
Member

@Steffan153 You can add the suggestions to a batch and commit them all at once btw. You don't need to do each separately

@cgccuser cgccuser enabled auto-merge May 28, 2022 19:12
@cgccuser cgccuser merged commit 366ba0e into main May 28, 2022
@cgccuser cgccuser deleted the diagonals branch May 28, 2022 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Seggan Seggan Seggan approved these changes

@cgccuser cgccuser cgccuser approved these changes

@lyxal lyxal Awaiting requested review from lyxal

@chunkybanana chunkybanana Awaiting requested review from chunkybanana

Assignees
No one assigned
Labels
PR: Bug Fix PR only tag - a PR that is fixing a bug/issue PR: Element Implementation PR only tag - a PR that is implementing an element request. PR: Enhancement PR only tag - for enhancements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add element to get all anti-diagonals of matrix ÞD does not work correctly on non-square matrices
4 participants
@Steffan153 @cgccuser @Seggan @Vyxal-Bot-old