Skip to content

Fixed RegExp for a few countries - #11

Open
stefanomasini wants to merge 6 commits into
maielo:masterfrom
stefanomasini:master
Open

stefanomasini wants to merge 6 commits into
maielo:masterfrom
stefanomasini:master

Conversation

@stefanomasini

@stefanomasini stefanomasini commented Jul 23, 2024 •

Copy link
Copy Markdown

@stefanomasini stefanomasini changed the title fix Turkish Tax Identification Number (10 cyphers, not 9) Fixed RegExp for a few countries Jul 23, 2024
@maielo

maielo commented Jul 25, 2024

Copy link
Copy Markdown
Owner

Hello!

Thanks for the PR.

Was testing it and it seems the Mexican one does not work so I kept the @dwelle's one. For turkey, I see there are 10 digits and TIN also has 11 digits.

The Chile changed regex is too forgiving: \b[0-9]{2}\.[0-9]{3}\.[0-9]{3}-[0-9kK]\b this one should be it per: https://www.oecd.org/content/dam/oecd/en/topics/policy-issue-focus/aeoi/chile-tin.pdf

Also would be great to update examples so we can test it against it and let users know how it looks like

@stefanomasini

Copy link
Copy Markdown
Author

Thanks for your feedback.

I had a few thousand records to process with this library and the changes I submitted reflect what I had to do in order for it to work. I did a quick investigation on the ones that broke in my dataset and noticed that the regexps where not matching what was described in the links I posted.

This way I was able to process and manually fix a few records to make them match the updated regexps. Ultimately, the whole dataset went through Stripe with no issues, apart from a few obvious outliers, so I'm really happy about the outcome and I thank you very much for providing the library.

That said, I don't presume my data set to be comprehensive enough to represent a confirmation that my fix is 100% correct. I only used the updated regexp to reduce the number of those I would have needed to process by hand, and trusted Stripe to ultimately reject my work if the ids wouldn't comply. That didn't happen but, again, I only had a few thousand records.

Also, this was just a one shot job for me, so I'm offering my fixes as is. Unfortunately I don't have much time to work on this further.

See what you can do with this PR.
And thanks again!
Cheers
Stefano

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