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

Export withBoundedScientific #647

Closed
wants to merge 1 commit into from
Closed

Conversation

qrilka
Copy link

@qrilka qrilka commented Jun 13, 2018

http://hackage.haskell.org/package/aeson-1.4.0.0/docs/Data-Aeson.html#v:withScientific advises to use withBoundedScientific for unbounded types but the function wasn't exported

http://hackage.haskell.org/package/aeson-1.4.0.0/docs/Data-Aeson.html#v:withScientific advises to use `withBoundedScientific` for unbounded types but the function wasn't exported
@bergmark
Copy link
Collaborator

Thanks,

What is your use case for withBoundedScientific? I'm interested in knowing if exporting it is the best way forward or if there are other solutions we should encourage.

There are some things do do/discuss:

  • Adding bounds on a type during deserialization is not ideal if you control the underlying type. I think it's preferable to use a smart constructor in combination with withScientific instead (implementing the bounds check yourself is easy). Would that work for your use case?
  • Do people want a different bound? It's easy to parameterize it, so it's probably best to do that before adding the export.
  • Currently we only have an upper bound on the exponent. @phadej suggested we add a lower bound for converting to e.g. Decimal. A lower bound isn't currently needed inside aeson but adding it wouldn't affect the current instances.
  • If we export it, I think it should be exported from all modules that export withScientific.

@phadej
Copy link
Collaborator

phadej commented Oct 9, 2021

ping @qrilka

@qrilka
Copy link
Author

qrilka commented Oct 9, 2021

I don't have a particular use case for this and not quite remember the details lead to this PR, probably the motivation was to make haddocks consistent

@phadej phadej closed this Oct 9, 2021
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.

None yet

3 participants