Skip to content

Restrict swap_rows, hnf, snf to MatElem; to apply to MatRingElem work with underlying matrix#2339

Open
JohnAAbbott wants to merge 3 commits intoNemocas:masterfrom
JohnAAbbott:JAA/MatrixElem-swap-rows
Open

Restrict swap_rows, hnf, snf to MatElem; to apply to MatRingElem work with underlying matrix#2339
JohnAAbbott wants to merge 3 commits intoNemocas:masterfrom
JohnAAbbott:JAA/MatrixElem-swap-rows

Conversation

@JohnAAbbott
Copy link
Collaborator

Draft because I disabled swap_rows and swap_rows! for MatRingElem (by changing MatrixElem to MatElem, as suggested by @fingolfin in issue #2247 ), as a consequence I also disabled in the same way hnf (several variants) and snf (several variants). I have commented out the corresponding tests -- to state the obvious, these will be removed if the draft becomes a non-draft.
This is obviously a breaking change... so there's no need to state the obvious!

@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.08%. Comparing base (f9e624d) to head (4943eaa).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2339      +/-   ##
==========================================
- Coverage   88.15%   88.08%   -0.07%     
==========================================
  Files         126      126              
  Lines       32093    32079      -14     
==========================================
- Hits        28290    28258      -32     
- Misses       3803     3821      +18     

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

@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.14%. Comparing base (f9e624d) to head (4943eaa).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2339      +/-   ##
==========================================
- Coverage   88.15%   88.14%   -0.01%     
==========================================
  Files         126      126              
  Lines       32093    32091       -2     
==========================================
- Hits        28290    28288       -2     
  Misses       3803     3803              

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

1 similar comment
@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.14%. Comparing base (f9e624d) to head (4943eaa).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2339      +/-   ##
==========================================
- Coverage   88.15%   88.14%   -0.01%     
==========================================
  Files         126      126              
  Lines       32093    32091       -2     
==========================================
- Hits        28290    28288       -2     
  Misses       3803     3803              

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

@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.14%. Comparing base (f9e624d) to head (4207857).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2339      +/-   ##
==========================================
- Coverage   88.15%   88.14%   -0.01%     
==========================================
  Files         126      126              
  Lines       32093    32093              
==========================================
- Hits        28290    28288       -2     
- Misses       3803     3805       +2     

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

@JohnAAbbott
Copy link
Collaborator Author

Question:
Is the intention that the hnf and snf functions work also with MatRingElem (via simple 1-liner functions)? [if so, then I should re-activate the commented out tests]
And what about swap_rows?

@thofma
Copy link
Member

thofma commented Feb 10, 2026

My guess would be that we don't need hnf, snf or swap_rows methods for MatRingElem.

@fingolfin
Copy link
Member

No objections to this in triage.

@fingolfin fingolfin removed the triage label Feb 11, 2026
@JohnAAbbott JohnAAbbott marked this pull request as ready for review February 11, 2026 12:23
@JohnAAbbott JohnAAbbott changed the title MatrixElem to MatElem only; for swap_rows, hnf, snf MatrixElem to MatElem; only for swap_rows, hnf, snf Feb 11, 2026
@JohnAAbbott JohnAAbbott added the release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes label Feb 11, 2026
@fingolfin fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 11, 2026
@JohnAAbbott JohnAAbbott changed the title MatrixElem to MatElem; only for swap_rows, hnf, snf Restrict swap_rows, hnf, snf to MatElem; for MatRingElem apply to underlying matrix Feb 11, 2026
@JohnAAbbott JohnAAbbott changed the title Restrict swap_rows, hnf, snf to MatElem; for MatRingElem apply to underlying matrix Restrict swap_rows, hnf, snf to MatElem; to apply to MatRingElem work with underlying matrix Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants