agda / agda-stdlib

The Agda standard library
https://wiki.portal.chalmers.se/agda/Libraries/StandardLibrary
Other
585 stars 237 forks source link

Mess around with IsSemiringWithoutOne reexports #2499

Open Taneb opened 1 week ago

Taneb commented 1 week ago

It was missing several and had some duplicated, which was causing problems when trying to use those names

I don't know how to changelog this

MatthewDaggitt commented 1 week ago

I don't know how to changelog this

I would document this as a bug fix. Noting the removal of the Carrier export and then listing the new exports.

jamesmckinna commented 1 week ago

As the author of the prior, now offending, commit, I drew attention in the discussion comments on the relevant PR to the possibility of having introduced such a bug...

... having said that, the setoid export was for a reason, and I think that the revisions here revert the relevant consequence (namely to export negated equality...), so I think I'd prefer it if instead

There's a meta-issue here:

This is not to throw shade either on me (for a buggy commit), or @Taneb for the fix here, but to point out that, wittingly or unwittingly, reverting prior commits may have further subsequent negative downstream consequences.

Happy to be corrected if I've misconstrued the situation here!

jamesmckinna commented 1 week ago

Allow me to correct myself, after yesterday's splurge/rant, for which I apologise. I did some more digging through the history, and the various import/export dependencies...

Summary:

Hope this helps! Apologies for interruptions in the smooth running of things...

jamesmckinna commented 1 week ago

Suggest removing the export of setoid completely... but not (yet) sure what the knock-on consequences of that might be...

Ugh. Worse that I thought: the uniqueness properties of inverse, defined as manifest fields, in Algebra.Structures.IsGroup, rely on Algebra.Consequences.Setoid, which depends, as the name suggests, on having a Bundled Setoid, rather than the underlying IsEquivalence for access to the properties of equality. So following my suggestion (to have IsMagma not export a setoid field) would actually have a larger scale knock-on effect.

Nevertheless, I think deleting the setoid export from IsSemiringWithoutOne should be harmless?

MatthewDaggitt commented 4 days ago

Gah these algebra imports, re-exports are always nightmarish. I'm not across the exact details, but my understanding aligns with both @jamesmckinna and @Taneb in that we try to avoid structures exporting bundles.