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

Change the structure of the GenT monad, to better track explanations. #4838

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

TimSheard
Copy link
Contributor

@TimSheard TimSheard commented Jan 17, 2025

The GenT monad is now a reader of not just Mode, but of both Mode and [NE.NonEmpty String]

The GE monad constructors types have changed
from

data GE a
  = FatalError [NE.NonEmpty String] (NE.NonEmpty String)
  | GenError [NE.NonEmpty String] (NE.NonEmpty String)
  | Result [NE.NonEmpty String] a

To

data GE a
  = FatalError [NE.NonEmpty String]
  | GenError [NE.NonEmpty String]
  | Result a

In addition to these changes in the module Constrained.GenT, several other modules have had their error messages slightly changed so that they display better using the new monad

Checklist

  • Commits in meaningful sequence and with useful messages
  • Tests added or updated when needed
  • CHANGELOG.md files updated for packages with externally visible changes

    New section is never added with the code changes. (See RELEASING.md)
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary

    If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • Code formatted (use scripts/fourmolize.sh)
  • Cabal files formatted (use scripts/cabal-format.sh)
  • hie.yaml updated (use scripts/gen-hie.sh)
  • Self-reviewed the diff

@TimSheard TimSheard force-pushed the ts-NewGenTmonad branch 4 times, most recently from da6f6e2 to 1949778 Compare January 21, 2025 21:11
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Just a few preliminary comments

libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
@TimSheard TimSheard marked this pull request as ready for review January 22, 2025 23:12
@TimSheard TimSheard requested a review from a team as a code owner January 22, 2025 23:12
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Although I don't fully understand the change to GenT and why it is necessary it looks like a very nice cleanup and improvement, especially to GE type.
I'll take a closer look at some point into how GenT is being used to convince myself that it is indeed the way it suppose to be.

libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/test/Constrained/Test.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
libs/constrained-generators/src/Constrained/GenT.hs Outdated Show resolved Hide resolved
Defined pickAll the basis of sums with fixed length.
Added getSizedList as a method of the Foldy class
getSizeList cost is metered at 1000 calls. Typical calls are less than 10.
Gave HasSpec (optional) method 'typeSpecHasError' a better default value
@TimSheard TimSheard force-pushed the ts-NewGenTmonad branch 2 times, most recently from 11819da to 5bdb6a9 Compare January 24, 2025 06:00
@TimSheard
Copy link
Contributor Author

Finally

@TimSheard TimSheard merged commit 9d65b0d into master Jan 24, 2025
153 of 156 checks passed
@TimSheard TimSheard deleted the ts-NewGenTmonad branch January 24, 2025 16:19
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.

2 participants