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

NiceMonomorphism regression #5037

Open
1 of 5 tasks
fingolfin opened this issue Sep 7, 2022 · 4 comments
Open
1 of 5 tasks

NiceMonomorphism regression #5037

fingolfin opened this issue Sep 7, 2022 · 4 comments
Assignees
@ThomasBreuer
Labels
regression A bug that only occurs in the branch, not in a release topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker)

Comments

@fingolfin
Copy link
Member

fingolfin commented Sep 7, 2022 •
edited by ThomasBreuer
Loading

Unfortunately PR #5029 causes a regression in a bunch of packages, see this report:

I think we need to disable this error for now -- or perhaps turn it into an InfoWarning, but the warning must not be shown on the default level as that would still break those test suites).

On the long run, someone can submit PRs to each of those packages to fix them up, and once they all had a release, we can re-enable the error.

[ IMHO this also answers whether we should backport that PR ;-) ]
@fingolfin fingolfin added regression A bug that only occurs in the branch, not in a release topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker) labels Sep 7, 2022
@ThomasBreuer
Copy link
Contributor

I am not surprised about these problems, I was rather surprised that the problems with the previous code (infinite recursion when calling Kernel) were not observed earlier.

I am still convinced that #5029 is a reasonable bugfix. (And the "regression" label looks funny from this point of view.)
Replacing the error message by a warning that is not shown does not look sensible to me.

For the moment, I think that reverting the changes from #5029 is the only way to avoid the failures in the tests of the packages.
Adjusting these packages to the intended changes is technically not difficult. Let us see how this goes on.

@fingolfin fingolfin mentioned this issue Sep 8, 2022
Merged
ThomasBreuer added a commit to ThomasBreuer/gap that referenced this issue Sep 8, 2022
@ThomasBreuer
for the moment, omit the test of the IsInjective flag
cbd5090 
This addresses issue gap-system#5037.
The test will be activated again as soon as the relevant packages
are adjusted to it.
@ThomasBreuer ThomasBreuer mentioned this issue Sep 8, 2022
Merged
fingolfin pushed a commit that referenced this issue Sep 9, 2022
@ThomasBreuer @fingolfin
for the moment, omit the test of the IsInjective flag
d502333 
This addresses issue #5037.
The test will be activated again as soon as the relevant packages
are adjusted to it.
@fingolfin
Copy link
Member Author

All of these are resolved by now, so I guess we could re-enable the strict check for this

@ThomasBreuer
Copy link
Contributor

ThomasBreuer commented Oct 27, 2022 •
edited
Loading

The problem is fixed in the development versions of the packages in question, but not yet in released versions.
Currently the latest released versions of groupoids (1.71), rcwa (4.7.0), and Semigroups (5.0.2) do not contain the fix.

@fingolfin
Copy link
Member Author

I believe all packages had updates with the fixes in them by now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ThomasBreuer ThomasBreuer

Labels
regression A bug that only occurs in the branch, not in a release topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fingolfin @ThomasBreuer