Skip to content

Conversation

@LilithHafner
Copy link
Member

#46508 is based on the (false) assumption that defs is always presorted. I hope this PR makes that assumption true.

@thofma, does this fix the error when testing Hecke?

@thofma
Copy link
Contributor

thofma commented Sep 25, 2022

Indeed this fixes the problem! Thanks for the quick fixup. Is it possible to add a test? (I don't know how easy it is to cook up a small example exhibiting this bug.)

@LilithHafner
Copy link
Member Author

I don't know how I would go about testing this.

@thofma
Copy link
Contributor

thofma commented Sep 26, 2022

Then I guess this is good to go :)

@KristofferC KristofferC added the merge me PR is reviewed. Merge when all tests are passing label Sep 30, 2022
@LilithHafner LilithHafner merged commit 4db5ffa into master Sep 30, 2022
@LilithHafner LilithHafner deleted the fixup-46508 branch September 30, 2022 12:20
KristofferC pushed a commit that referenced this pull request Sep 30, 2022
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Oct 1, 2022
LilithHafner pushed a commit that referenced this pull request Oct 2, 2022
KristofferC pushed a commit that referenced this pull request Oct 3, 2022
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.

5 participants