-
-
Notifications
You must be signed in to change notification settings - Fork 50
add pattern and test for greek license plates #69
Conversation
denisorehovsky
left a comment
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.
Thanks for the PR! Looks good. Just a few comments to look into here.
| 'FR': r'(^[A-Z]{2}-\d{3}-[A-Z]{2}$)|(^\d{1,4}\s[A-Z]{1,3}\s\d{2}$)' | ||
| 'FR': r'(^[A-Z]{2}-\d{3}-[A-Z]{2}$)|(^\d{1,4}\s[A-Z]{1,3}\s\d{2}$)', | ||
| # Regex pattern to match Greek license plates | ||
| 'GR': r'(^[ABEZHIKMNOPTYXΑΒΕΖΗΙΚΜΝΟΡΤΥΧ]{3}-\d{4})', |
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.
Can you clarify please why did you repeat ABEZHIKMNOPTYX two times here?
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.
The two groups looks the same but the second contains greek characters in order to detect greek licence plate with greek characters.
>>> a = 'ABEZHIKMNOPTYXΑΒΕΖΗΙΚΜΝΟΡΤΥΧ'
>>> for latin_char, greek_char in zip(a[:14], a[14:]):
... print(latin_char, ord(latin_char), greek_char, ord(greek_char))
...
A 65 Α 913
B 66 Β 914
E 69 Ε 917
Z 90 Ζ 918
H 72 Η 919
I 73 Ι 921
K 75 Κ 922
M 77 Μ 924
N 78 Ν 925
O 79 Ο 927
P 80 Ρ 929
T 84 Τ 932
Y 89 Υ 933
X 88 Χ 935There 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.
Oh, ok. Thanks. Greek characters are not visible to me.
Codecov Report
@@ Coverage Diff @@
## master #69 +/- ##
=======================================
Coverage 96.07% 96.07%
=======================================
Files 4 4
Lines 51 51
=======================================
Hits 49 49
Misses 2 2
Continue to review full report at Codecov.
|
|
@lk-geimfari There is something wrong with our CI. I think we can merge this guy but then create another PR with fixes. |
|
@apirobot Feel free to manage this repository. Merge it! |
No description provided.