Handle human readable number casting in general #2797

Aditramesh · 2023-04-10T05:12:34Z

Adds functionalities to handle casting of text to integer and decimal types which was implemented for numeric types in #1355

Technical details
Uses similar logic for the integer types and added functions for decimal types(real ,double precision ) to work with the logic for numeric

Screenshots

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the develop branch of the repository
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no
visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

…eneral

…ting_in_general' into handle_human_readable_number_casting_in_general

…eneral

dmos62 · 2023-04-11T09:02:14Z

Continuation of #2465

mathemancer

There's an issue with the integer-finding regex that I found.

Also, please remember to write some tests. Parsing regexes by eyeball is not a robust way for me to check that these have the correct behavior, and we won't be merging this PR without a decent test suite associated with it.

Thank you for your effort so far.

mathemancer · 2023-04-11T14:55:45Z

db/types/operations/cast.py

+    single_digit = r"^[0-9]$"
+    no_separator = r"[0-9]{2,}(?:([,])[0-9]{1,2}|[0-9]{4,})?"


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.

I'm also not sure what the no_separator part should match at this point.

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]+)?"

You don't need the beginning or end string markers (or the [-] in the individual expression. Those are handled by the wrapping expression.

I have made those changes and added tests,thanks for the help throughout @mathemancer .

…ps://github.com/Aditramesh/mathesar into handle_human_readable_number_casting_in_general update branch

…eneral

mathemancer

Thank you for writing some test cases. It really helps me see what's going on at a glance. However, there's been a regression in the logic since my last review. Please see the specific comment.

Thanks for your effort on this so far!

mathemancer · 2023-04-28T04:40:07Z

db/types/operations/cast.py

+    no_separator = r"\d+"
+    comma_or_period_separator = r"([0-9]|[0-9]{3})(([,.])[0-9]{3})+"
+    comma_separator_lakh_system = r"([0-9]{1,2}(?:(,)[0-9]{2})+),[0-9]{3}"
+    space_separator = r"[0-9]{1,3}(?:( )[0-9]{3})+"
+    single_quote_separator = r"([0-9]{1,3})(?:(\'')[0-9]{3})+(?:(\'')?[0-9]*)?"


For the comma and period separator ones, we need at least 2 groups of three to be sure that it's an integer. This is crucial in our inference system at the moment. I.e., it's not possible to determine the value of 1,234 without knowing the locale.

oh sorry my bad ,changing the regex to
comma_or_period_separator = r"([0-9]|[0-9]{3})(([,.])[0-9]{3}){2,}" should fix the issue you are referring to,correct ?

…ps://github.com/Aditramesh/mathesar into handle_human_readable_number_casting_in_general

github-actions · 2023-06-14T21:21:06Z

This pull request has not been updated in 45 days and is being marked as stale. It will automatically be closed in 30 days if not updated by then.

rajatvijay · 2023-06-16T05:48:44Z

Closing this due to inactivity.

Aditramesh · 2023-06-19T11:28:47Z

@rajatvijay the requested changed were made the pr has to be checked once and merged

github-actions · 2023-08-10T21:20:54Z

This pull request has not been updated in 45 days and is being marked as stale. It will automatically be closed in 30 days if not updated by then.

github-actions · 2023-09-30T21:21:05Z

This pull request has not been updated in 45 days and is being marked as stale. It will automatically be closed in 30 days if not updated by then.

mathemancer · 2023-10-11T07:20:52Z

I'm sorry, @Aditramesh . This still isn't working correctly, and I've run out of time for going back-and-forth on this PR. I'm closing it.

I should have done this awhile ago.

Aditramesh and others added 13 commits February 13, 2023 19:40

fixes 1309

17b3124

fixes 1309

50608e2

fixes1309

b598720

fixes1309

2b7d910

Merge branch 'develop' into handle_human_readable_number_casting_in_g…

9dde06b

…eneral

add regexes and decimal cast functions

22e952f

fixed indentation

abfec31

fixed linting

cf051c9

fixed linting

5ecf7cf

fixed a small error

9ede0a7

Merge branch 'develop' into handle_human_readable_number_casting_in_g…

877eb0a

…eneral

added functions to support casting integers and decimals

6c0d143

Merge remote-tracking branch 'origin/handle_human_readable_number_cas…

d30eea3

…ting_in_general' into handle_human_readable_number_casting_in_general

rajatvijay requested a review from mathemancer April 10, 2023 15:07

rajatvijay assigned mathemancer Apr 10, 2023

rajatvijay added the pr-status: review A PR awaiting review label Apr 10, 2023

rajatvijay added this to the Backlog milestone Apr 10, 2023

Merge branch 'develop' into handle_human_readable_number_casting_in_g…

e971d00

…eneral

mathemancer requested changes Apr 11, 2023

View reviewed changes

rajatvijay assigned Aditramesh and unassigned mathemancer Apr 12, 2023

rajatvijay added pr-status: revision A PR awaiting follow-up work from its author after review and removed pr-status: review A PR awaiting review labels Apr 12, 2023

Aditramesh added 4 commits April 15, 2023 11:00

fixed integer regex's

af95537

added tests to verify integer casting

12f151b

Merge branch 'handle_human_readable_number_casting_in_general' of htt…

992c2fb

…ps://github.com/Aditramesh/mathesar into handle_human_readable_number_casting_in_general update branch

removed comment

08a8840

kgodey added pr-status: review A PR awaiting review and removed pr-status: revision A PR awaiting follow-up work from its author after review labels Apr 18, 2023

kgodey unassigned Aditramesh Apr 18, 2023

Merge branch 'develop' into handle_human_readable_number_casting_in_g…

7841fcb

…eneral

mathemancer self-requested a review April 26, 2023 13:28

mathemancer requested changes Apr 28, 2023

View reviewed changes

rajatvijay assigned Aditramesh and unassigned mathemancer Apr 28, 2023

rajatvijay added pr-status: revision A PR awaiting follow-up work from its author after review and removed pr-status: review A PR awaiting review labels Apr 28, 2023

Aditramesh added 2 commits April 30, 2023 19:48

fixed integer regexs

2d650cd

Merge branch 'handle_human_readable_number_casting_in_general' of htt…

33a4f4e

…ps://github.com/Aditramesh/mathesar into handle_human_readable_number_casting_in_general

github-actions bot added the stale label Jun 14, 2023

rajatvijay closed this Jun 16, 2023

kgodey reopened this Jun 26, 2023

kgodey assigned mathemancer and unassigned Aditramesh Jun 26, 2023

kgodey added pr-status: review A PR awaiting review and removed stale pr-status: revision A PR awaiting follow-up work from its author after review labels Jun 26, 2023

github-actions bot added the stale label Aug 10, 2023

rajatvijay removed the stale label Aug 16, 2023

rajatvijay requested a review from mathemancer August 16, 2023 09:43

github-actions bot added the stale label Sep 30, 2023

rajatvijay removed the stale label Oct 2, 2023

mathemancer closed this Oct 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle human readable number casting in general #2797

Handle human readable number casting in general #2797

Aditramesh commented Apr 10, 2023 •

edited

Loading

dmos62 commented Apr 11, 2023

mathemancer left a comment

mathemancer Apr 11, 2023

mathemancer Apr 11, 2023

Aditramesh Apr 12, 2023

mathemancer Apr 12, 2023

Aditramesh Apr 15, 2023

mathemancer left a comment

mathemancer Apr 28, 2023

Aditramesh Apr 28, 2023

github-actions bot commented Jun 14, 2023

rajatvijay commented Jun 16, 2023

Aditramesh commented Jun 19, 2023

github-actions bot commented Aug 10, 2023

github-actions bot commented Sep 30, 2023

mathemancer commented Oct 11, 2023

		single_digit = r"^[0-9]$"
		no_separator = r"[0-9]{2,}(?:([,])[0-9]{1,2}\|[0-9]{4,})?"

Handle human readable number casting in general #2797

Handle human readable number casting in general #2797

Conversation

Aditramesh commented Apr 10, 2023 • edited Loading

Checklist

Developer Certificate of Origin

dmos62 commented Apr 11, 2023

mathemancer left a comment

Choose a reason for hiding this comment

mathemancer Apr 11, 2023

Choose a reason for hiding this comment

mathemancer Apr 11, 2023

Choose a reason for hiding this comment

Aditramesh Apr 12, 2023

Choose a reason for hiding this comment

mathemancer Apr 12, 2023

Choose a reason for hiding this comment

Aditramesh Apr 15, 2023

Choose a reason for hiding this comment

mathemancer left a comment

Choose a reason for hiding this comment

mathemancer Apr 28, 2023

Choose a reason for hiding this comment

Aditramesh Apr 28, 2023

Choose a reason for hiding this comment

github-actions bot commented Jun 14, 2023

rajatvijay commented Jun 16, 2023

Aditramesh commented Jun 19, 2023

github-actions bot commented Aug 10, 2023

github-actions bot commented Sep 30, 2023

mathemancer commented Oct 11, 2023

Aditramesh commented Apr 10, 2023 •

edited

Loading