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

Add float (or finite) type #1960

Closed
davetapley opened this issue Sep 19, 2022 Discussed in #1959 · 2 comments · Fixed by #2052
Closed

Add float (or finite) type #1960

davetapley opened this issue Sep 19, 2022 Discussed in #1959 · 2 comments · Fixed by #2052
Labels
enhancement Possible enhancement has PR A Pull Request to fix the issue is available help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: easy

Comments

@davetapley
Copy link
Contributor

Discussed in #1959

Originally posted by davetapley September 15, 2022
Since we already have an integer type:

export const integer: ISimpleType<number> = new CoreType<number, number, number>(
"integer",
TypeFlags.Integer,
(v) => isInteger(v)
)

Do we want one or both of these?

  1. A float type, which would restrict against NaN.
  2. A finite type, which would restrict against NaN and infinities (taking inspiration from isFinite).

I'll concede that both could be implemented with refinement, but so could integer and we already have that.

This would have saved me from a bug where I am using parseFloat and a NaN was sneaking in to my model.
It would be great if I could just specify float in my model and have MST throw, instead of having to check for NaN manually.


Happy to make a PR if it will be accepted 🤞🏻

@coolsoftwaretyler
Copy link
Collaborator

Hey @davetapley - I know it's been a while. I can't answer whether or not a PR would be accepted, but I agree this makes sense to do. I can ask around the maintainers and try to get a sense of what they're thinking in terms of adding functionality like this. Are you still interested in writing a PR for it? If not, no worries, I know it's been quite a while since you brought this up. I just wanted you to at least get an acknowledgment here, albeit belated.

@coolsoftwaretyler
Copy link
Collaborator

For folks who aren't following the discussion, we are open for PRs to add new types! Here's the thread: #1959

I'll leave this open for Dave to attempt a PR if they're still interested. But if not, I'll take this on myself and close out the issue once it's all set.

@coolsoftwaretyler coolsoftwaretyler added enhancement Possible enhancement help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: easy labels Jun 25, 2023
@coolsoftwaretyler coolsoftwaretyler added the has PR A Pull Request to fix the issue is available label Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Possible enhancement has PR A Pull Request to fix the issue is available help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: easy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants