-
-
Notifications
You must be signed in to change notification settings - Fork 316
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
Handle human readable number casting in general #2797
Closed
Aditramesh
wants to merge
21
commits into
mathesar-foundation:develop
from
Aditramesh:handle_human_readable_number_casting_in_general
Closed
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
17b3124
fixes 1309
Aditramesh 50608e2
fixes 1309
Aditramesh b598720
fixes1309
Aditramesh 2b7d910
fixes1309
Aditramesh 9dde06b
Merge branch 'develop' into handle_human_readable_number_casting_in_g…
mathemancer 22e952f
add regexes and decimal cast functions
Aditramesh abfec31
fixed indentation
Aditramesh cf051c9
fixed linting
Aditramesh 5ecf7cf
fixed linting
Aditramesh 9ede0a7
fixed a small error
Aditramesh 877eb0a
Merge branch 'develop' into handle_human_readable_number_casting_in_g…
rajatvijay 6c0d143
added functions to support casting integers and decimals
Aditramesh d30eea3
Merge remote-tracking branch 'origin/handle_human_readable_number_cas…
Aditramesh e971d00
Merge branch 'develop' into handle_human_readable_number_casting_in_g…
dmos62 af95537
fixed integer regex's
Aditramesh 12f151b
added tests to verify integer casting
Aditramesh 992c2fb
Merge branch 'handle_human_readable_number_casting_in_general' of htt…
Aditramesh 08a8840
removed comment
Aditramesh 7841fcb
Merge branch 'develop' into handle_human_readable_number_casting_in_g…
mathemancer 2d650cd
fixed integer regexs
Aditramesh 33a4f4e
Merge branch 'handle_human_readable_number_casting_in_general' of htt…
Aditramesh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need separate expressions for single digit and multi digit non-separated integers. An integer with no separator is just a sequence of digits. No other complexity needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also not sure what the
no_separator
part should match at this point.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought numbers like 12,345 also have to be matched,that's why i was writing those two regex's ,later i realised 12,345 should not be matched so i think these the regex's should look like this now
no_separator = r"^-?\d+$"
comma_separator = r"[0-9]{1,3}(?:(,)[0-9]{3}){2,}"
period_separator = r"[0-9]{1,3}(?:(.)[0-9]{3}){2,}"
comma_separator_lakh_system = r"[0-9]{1,2}(?:(,)[0-9]{2})+,[0-9]{3}?"
single_quote_separator = r"[0-9]{1,3}(?:('')[0-9]{3})+"
space_separator = r"[0-9]{1,3}(?:( )[0-9]{3})+(?:([,])[0-9]+)?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need the beginning or end string markers (or the
[-]
in the individual expression. Those are handled by the wrapping expression.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made those changes and added tests,thanks for the help throughout @mathemancer .