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.

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

[ refactor ] Add Data.Nat.SumAndProduct #2558

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jamesmckinna
Copy link
Contributor

@jamesmckinna jamesmckinna commented Jan 16, 2025

Implements the 'provisional' v2.3 fix for #2553 .

NB.:

  • starting from a blank CHANGELOG in anticipation of Update documentation from v2.2 release version to v2.3-dev #2557 being merged
  • fixes a spurious deprecation message from v2.0 in Data.List.Properties concerning sum-++-commute, but now the message is... anachronistic. Wasn't sure how to deal with this properly!
  • can't seem to understand why the deprecation warning is issued when checking Data.Nat.Primality.Factorisation when generating the HTML index, but not when testing stdlib! Nor why it is being generated... but I may have forgotten the fine(r) points about how this works... :-( UPDATED: seemingly FIXED now!?

@Taneb
Copy link
Member

Taneb commented Jan 17, 2025

I have to say, I don't like the module name... there are other monoids on Nat (e.g. (max, 0)) whose folds we'd also want to have in the same module if we have them.

@jamesmckinna
Copy link
Contributor Author

jamesmckinna commented Jan 17, 2025

Thanks @Taneb !

Bear in mind that I regard this PR as provisional, indeed minimal wrt not refactoring to lift out the actual Monoid abstractions as such, before a 'proper' (breaking) refactoring which will deprecate/remove this module, and all traces of the original definitions. In particular, I would want this PR only to emulate existing functionality (but product-++ seemed too tempting not to add, emphasising the need to abstract the constructions...?), rather than try to generalise it to arbitrary monoid structures arising on Nat. Perhaps that's a failure of imagination/nerve?

Eg, I could even imagine doing that generalisation (in whatever form) downstream, and then simply deprecating this module with its 'clunky' name. perhaps I even thought that would be inevitable, so either didn't care, or actively chose, to make the name 'clunky'... on a 'build one to throw away' principle...

That said, would (any of) the following be better candidate names?

  • Data.Nat.MonoidAction/Data.Nat.FreeMonoidAction (a bit too general, since we're only considering the action of Nat on itself...?); it's a pity I haven't/hadn't managed to finish Add Algebra.Action.* and friends #2350 before this, but I've been trying to not let the 'perfect design' get in the way of progress... ;-), but eventually, as with Refactor Data.Nat.GeneralisedArithmetic #2446 , there ought to be a grand refactoring of all this stuff...
  • Data.Nat.Monoid

Other suggestions welcome! @JacquesCarette any thoughts on this?

@JacquesCarette
Copy link
Contributor

As I said elsewhere, fully agree with moving these out of where they currently are.

Hmm, why Data.Nat.* though? I was thinking of these as "special cases of List-based computation" and so under Data.List. Data.List.NatActions? [Obviously not wedded to that awkward name, but pushing things along.]

If it were in Data.Nat then I'd see it being way more general, i.e. over any foldable container.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants