Skip to content

Conversation

@john-science
Copy link
Member

@john-science john-science commented Nov 11, 2025

What is the change? Why is it being made?

The function tryPickleOnAllContents3() hides all errors, so I do not see any value in it. So this PR removes it.

close #2356

SCR Information

Change Type: trivial

One-Sentence Description: This feature appears to me to be broken, so I am removing it.
One-Sentence Rationale: This feature appears to me to be broken, so I am removing it.

One-line Impact on Requirements: NA


Checklist

@john-science john-science added the cleanup Code/comment cleanup: Low Priority label Nov 12, 2025
@john-science
Copy link
Member Author

john-science commented Nov 12, 2025

@keckler @drewj-tp Do you guys know of any reason why this would be a bad idea? The function looks broken to me.

(I have obviously done all the downstream testing I can, and it all passes.)

@keckler
Copy link
Member

keckler commented Nov 12, 2025

I don't really know anything about that function. I have never used it, or even knew of its existence.

@john-science john-science marked this pull request as ready for review November 12, 2025 16:07
Copy link
Member

@opotowsky opotowsky left a comment

Choose a reason for hiding this comment

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

🩷🧼

@john-science john-science merged commit 05acca9 into main Nov 18, 2025
12 checks passed
@john-science john-science deleted the tryPickleOnAllContents3 branch November 18, 2025 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Code/comment cleanup: Low Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tryPickleOnAllContents3 passes all PicklingErrors

4 participants