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

Make number of decimal places for float serialisation configurable #28

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

georgefst
Copy link
Collaborator

(This is a mess, but I figured I'd see what you think before I go any further...)

Currently serialisation is hardcoded to always use six decimal places, which is often excessive. For my use case, it would be really great to get this down.

Unfortunately this change is very pervasive. And this is probably only about half of it (see hardcodedPrecision). I feel I ought to take a step back and try to understand the library architecture better to see if there may be a more elegant solution. Perhaps some refactoring is in order first - for example things like ParseableAttribute and parseIn have clearly outgrown their original purpose (not a criticism of you - I see that code predates the fork).

@lemmih
Copy link
Member

lemmih commented Jan 4, 2021

I like it.

@georgefst
Copy link
Collaborator Author

Great, I'll finish it off then.

Do you reckon it's worth using a record:

data SerializationOpts = SerializationOpts
    { floatPrecision :: Int
    }

?

Would be more descriptive than a plain Int argument, and make it easier to add other configuration. But then I don't know if there are any other options one would ever want.

@lemmih
Copy link
Member

lemmih commented Jan 4, 2021

Maybe prettyPrint :: Bool and indentation :: Int would be possible future options. In any case, I like the idea of using a record.

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