Add isalpha, istitle, and title APIs in str by Agent-Hellboy · Pull Request #2357 · lcompilers/lpython

Agent-Hellboy · 2023-10-02T17:43:21Z

No description provided.

certik · 2023-10-02T18:45:41Z

@Agent-Hellboy thanks, looks good. Make sure to update reference tests using ./run_tests -u.

integration_tests/test_str_01.py

src/runtime/lpython_builtin.py

Thirumalai-Shaktivel · 2023-10-03T04:22:49Z

src/runtime/lpython_builtin.py


+@overload
+def _lpython_str_isalpha(s: str) -> bool:
+    char: str


Here is the cause for the CI failure: #2359

I think renaming it or fixing the bug would resolve the CI failure.

Thirumalai-Shaktivel · 2023-10-03T04:23:14Z

Otherwise, this PR looks great!

Thirumalai-Shaktivel · 2023-10-03T04:56:00Z

@Agent-Hellboy you can try cd integration_tests && ./run_tests.py -b c to test c backend locally.

Agent-Hellboy · 2023-10-03T04:57:32Z

@Agent-Hellboy you can try cd integration_tests && ./run_tests.py -b c to test c backend locally.

okay , sure

Thirumalai-Shaktivel · 2023-10-03T05:50:15Z

I'm not sure what's causing the CI failure. Try debugging it logically and create an issue for it.
You can use lpython a.py --show-c to view the c code and lpython a.py --backend=c to compile LPython using the C backend.

Agent-Hellboy · 2023-10-06T01:21:22Z

I'm not sure what's causing the CI failure. Try debugging it logically and create an issue for it. You can use lpython a.py --show-c to view the c code and lpython a.py --backend=c to compile LPython using the C backend.

sure, I will try to do it today

Agent-Hellboy · 2023-10-08T04:46:00Z

@Thirumalai-Shaktivel, how can I link libasr library I generated main.c which has included some header file form libasr, but i don't know how to link libasr to compile it with gcc.

Thirumalai-Shaktivel · 2023-10-08T04:55:04Z

Can you please share some details of where you are trying to link libasr?

Thirumalai-Shaktivel · 2023-10-08T04:57:15Z

If you are trying to compile C code generated using --show-c, try specifying --backend=c in lpython. It links all the libraries.
Like, lpython a.py --backend=c will compile the generated c code using gcc or clang.

Agent-Hellboy · 2023-10-08T04:58:41Z

If you are trying to compile C code generated using --show-c, try specifying --backend=c in lpython. It links all the libraries.

okay

certik · 2023-10-08T06:52:39Z

Once it is ready for review, go ahead and click on the "Ready for review" button.

certik · 2023-10-08T12:44:19Z

src/runtime/lpython_builtin.py

        res += s + lis[i]
    return res

+@overload


Why do these need @overload? I think they are not overloaded, are they?

certik

I think this is fine. I left one minor comment.

@Thirumalai-Shaktivel would you mind please reviewing this as well, to make sure I didn't miss anything? If it looks good, you can merge it.

integration_tests/test_str_01.py

src/runtime/lpython_builtin.py

integration_tests/test_str_01.py

Thirumalai-Shaktivel · 2023-10-08T14:25:02Z

integration_tests/test_str_01.py

+    res: bool = a.istitle()
+    res2: bool = b.istitle()
+    assert res == True
+    assert res2 == False 


Add test for empty string.
Also add " Hello", "HeLlo" or something similar

Thanks, this was an edge case
CPython istitle and title is not clear in documentation
updated the code according to the response from cpython

>>> l="\tHello" >>> l.istitle() True >>> l="\thello" >>> l.istitle() False

Thirumalai-Shaktivel · 2023-10-08T14:25:49Z

Rest looks great, I left minor improvements.
Also, we have to resolve the conflicts.

Thirumalai-Shaktivel · 2023-10-09T05:20:07Z

src/runtime/lpython_builtin.py

+<<<<<<< HEAD
+<<<<<<< HEAD
+        elif ch.isalpha() and (ord('A') <= ord(ch) and ord(ch) <= ord('Z')):
+=======
+        elif ch.isalpha() and ch.isupper():
+>>>>>>> 5c5bcc620 (Fix merge conflict)
+=======
+        elif ch.isalpha() and (ord('A') <= ord(ch) and ord(ch) <= ord('Z')):
+>>>>>>> f7fcb9835 (Fix test references)
+            if word_start:
+                word_start = False
+            else:
+                return False  # Found an uppercase character in the middle of a word
+<<<<<<< HEAD
+<<<<<<< HEAD
+        elif ch.isalpha() and (ord('a') <= ord(ch) and ord(ch) <= ord('z')):
+=======
+        elif ch.isalpha() and ch.islower():
+>>>>>>> 5c5bcc620 (Fix merge conflict)
+=======
+        elif ch.isalpha() and (ord('a') <= ord(ch) and ord(ch) <= ord('z')):
+>>>>>>> f7fcb9835 (Fix test references)
+            if word_start:
+                return False  # Found a lowercase character in the middle of a word
+            word_start = False
+        else:
+            word_start = True
+
+    return True
+
+
+<<<<<<< HEAD
+<<<<<<< HEAD
+=======
+
+>>>>>>> 5c5bcc620 (Fix merge conflict)
+=======
+>>>>>>> f7fcb9835 (Fix test references)


Remove merge conflict tags

Suggested change

<<<<<<< HEAD

<<<<<<< HEAD

elif ch.isalpha() and (ord('A') <= ord(ch) and ord(ch) <= ord('Z')):

=======

elif ch.isalpha() and ch.isupper():

>>>>>>> 5c5bcc620 (Fix merge conflict)

=======

elif ch.isalpha() and (ord('A') <= ord(ch) and ord(ch) <= ord('Z')):

>>>>>>> f7fcb9835 (Fix test references)

if word_start:

word_start = False

else:

return False # Found an uppercase character in the middle of a word

<<<<<<< HEAD

<<<<<<< HEAD

elif ch.isalpha() and (ord('a') <= ord(ch) and ord(ch) <= ord('z')):

=======

elif ch.isalpha() and ch.islower():

>>>>>>> 5c5bcc620 (Fix merge conflict)

=======

elif ch.isalpha() and (ord('a') <= ord(ch) and ord(ch) <= ord('z')):

>>>>>>> f7fcb9835 (Fix test references)

if word_start:

return False # Found a lowercase character in the middle of a word

word_start = False

else:

word_start = True

return True

<<<<<<< HEAD

<<<<<<< HEAD

=======

>>>>>>> 5c5bcc620 (Fix merge conflict)

=======

>>>>>>> f7fcb9835 (Fix test references)

elif ch.isalpha() and (ord('A') <= ord(ch) and ord(ch) <= ord('Z')):

if word_start:

word_start = False

else:

return False # Found an uppercase character in the middle of a word

elif ch.isalpha() and (ord('a') <= ord(ch) and ord(ch) <= ord('z')):

if word_start:

return False # Found a lowercase character in the middle of a word

word_start = False

else:

word_start = True

return True

Agent-Hellboy · 2023-10-09T06:04:36Z

merge it for now, from now on, I will pick test cases from CPython
I have tested everything from https://github.com/Agent-Hellboy/cpython/blob/main/Lib/test/test_str.py#L696
https://github.com/Agent-Hellboy/cpython/blob/main/Lib/test/test_str.py#L734

Co-authored-by: Thirumalai Shaktivel <74826228+Thirumalai-Shaktivel@users.noreply.github.com>

Thirumalai-Shaktivel reviewed Oct 3, 2023

View reviewed changes

integration_tests/test_str_01.py Show resolved Hide resolved

Thirumalai-Shaktivel reviewed Oct 3, 2023

View reviewed changes

src/runtime/lpython_builtin.py Outdated Show resolved Hide resolved

Thirumalai-Shaktivel reviewed Oct 3, 2023

View reviewed changes

certik marked this pull request as draft October 8, 2023 06:52

Agent-Hellboy force-pushed the add_title_in_str branch from 542a169 to 31628a8 Compare October 8, 2023 09:43

Agent-Hellboy mentioned this pull request Oct 8, 2023

Handle isalpha and istitle like isupper, islower and several is* checks #2376

Open

Agent-Hellboy marked this pull request as ready for review October 8, 2023 10:10

certik reviewed Oct 8, 2023

View reviewed changes

certik requested a review from Thirumalai-Shaktivel October 8, 2023 12:44

certik approved these changes Oct 8, 2023

View reviewed changes