capitalize and upper functions (String module) #669

Madhav2310 · 2022-06-21T14:56:54Z

In continuation to #397

Madhav2310 · 2022-06-21T14:57:44Z

cc: @certik @czgdp1807 @namannimmo10 @faze-geek

certik · 2022-06-21T16:24:25Z

See the discussion here: https://gitlab.com/lfortran/lfortran/-/merge_requests/1752

I think we should add StringChr, StringOrd or something like that, and just use those for chr(). That is a builtin function, unrelated to this PR. But I wanted to point that out.

Madhav2310 · 2022-06-23T23:19:15Z

@czgdp1807 could you please help me fix this error?

czgdp1807 · 2022-06-24T09:51:46Z

integration_tests/test_string.py

+    test_capitalize()
+    test_upper()
+
+check() 


Add final new line.

czgdp1807 · 2022-06-24T09:53:09Z

integration_tests/test_string.py

+def test_capitalize():
+    i: str
+    i = capitalize("lpython")
+    assert i == "Lpython"


i = "lPythoN" assert capitalize(i) == "Lpython"

czgdp1807 · 2022-06-24T09:53:22Z

integration_tests/test_string.py

+    assert i == "Lpython"
+    i: str
+    i = capitalize("development")
+    assert i == "Development"


Similar for this.

czgdp1807 · 2022-06-24T09:53:41Z

integration_tests/test_string.py

+
+def test_upper():
+    i: str
+    i = upper("lpython")


Similar for this.

czgdp1807 · 2022-06-24T09:54:47Z

src/runtime/string.py

+    Return a copy of the string with its first character capitalized and the rest lowercased.
+    """
+    result: str
+    result = upper(s[0]) + s[1:]


The rest is not getting lower cased here I think. The rest is just picked up as is.

May be fill result character wise.

czgdp1807 · 2022-06-24T09:56:36Z

src/runtime/string.py

+    char : str
+
+    for char in s:
+        if ord(char) >= 97 and ord(char) <=122 :


Python allows character comparison. So you can directly do, char > 'a' and char < 'z'.

czgdp1807 · 2022-06-24T09:58:54Z

I think you have to register captialize, upper and the module string.py in https://github.com/lcompilers/lpython/blob/main/src/lpython/semantics/python_comptime_eval.h file. Just look how other functions are registered.

namannimmo10 · 2022-06-24T20:33:26Z

You can also maybe include string constants.

namannimmo10 · 2022-06-24T20:41:30Z

These functions are not available in CPython:

>>> from string import upper
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'upper' from 'string' (/Users/namannimmo/miniconda3/envs/lp/lib/python3.10/string.py)
>>>

That's why the integration tests won't pass. This would work as a method:

>>> "abc".upper()
'ABC'
>>> "abc".capitalize()
'Abc'
>>>

This reverts commit e07b0a0.

Madhav2310 · 2022-06-25T20:43:32Z

I think you have to register captialize, upper and the module string.py in https://github.com/lcompilers/lpython/blob/main/src/lpython/semantics/python_comptime_eval.h file. Just look how other functions are registered.

Should I just add these functions to python_builtin.py?
On looking more closely these string methods are all of str of python builtin functions: https://docs.python.org/3/library/stdtypes.html#string-methods

The string.py module has wholly different functions like format, parse, get_value etc. : https://docs.python.org/3/library/string.html

…egration tests made

Madhav2310 · 2022-07-08T05:39:24Z

@czgdp1807 I've tried adding upper, lower, and capitalize in python_comptime_eval.h. Please review and point me towards the next step

integration_tests/test_builtin_lower.py

…madhav6

integration_tests/test_builtin_upper.py

src/lpython/semantics/python_comptime_eval.h

src/runtime/lpython_builtin.py

src/runtime/string.py

Co-authored-by: Gagandeep Singh <gdp.1807@gmail.com>

…madhav6

namannimmo10 · 2022-07-08T09:43:12Z

integration_tests/test_builtin_capitalize.py

+
+def test_capitalize():
+    i: str
+    i = capitalize("lpyThon")


s: str
s = "lpyThon"
capitalize(s)

src/lpython/semantics/python_comptime_eval.h

certik · 2022-07-11T16:15:50Z

If I understand correctly, this implements both runtime and compile time capitalization? If so, we need to also add tests for both use cases.

Madhav2310 · 2022-07-12T14:16:13Z

If I understand correctly, this implements both runtime and compile time capitalization? If so, we need to also add tests for both use cases.

Yes, as soon as we get this working, I'll add the tests too

Madhav2310 · 2022-07-12T17:38:57Z

@czgdp1807 please check now, I think the python_comptime_eval.h code works now. I'm unsure why its shows an error in the complex1 test file, as I didn't make any changes whatsoever there. Thank you!

certik · 2022-10-05T11:37:38Z

@czgdp1807 what is the status of this, was this implemented?

certik · 2022-10-05T11:38:09Z

Obviously it is not finished, so I'll mark it as Draft for now.

czgdp1807 · 2022-10-05T12:32:41Z

Yes. I have added please take over label to it if someone wants to complete this.

Smit-create · 2022-11-09T04:58:39Z

Just the upper method is needed now. Will rebase this.

Harsh-Upadhayay · 2022-12-19T05:02:06Z

Hi, if this issue is still remaining then I'd like to take it over and solve it.
After reading the above conversation I have a vague idea that some of the bulitins of string library were implemented and the changes didn't passed integration tests.
Could someone please summaries what needs to be done and what was done.

capitalize and upper functions (String module)

90d928e

fixing

e07b0a0

czgdp1807 reviewed Jun 24, 2022

View reviewed changes

integration_tests/test_string.py Outdated

test_capitalize()

test_upper()

check()

Copy link

Collaborator

czgdp1807 Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add final new line.

czgdp1807 reviewed Jun 24, 2022

View reviewed changes

integration_tests/test_string.py Outdated

def test_upper():

i: str

i = upper("lpython")

Copy link

Collaborator

czgdp1807 Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar for this.

czgdp1807 reviewed Jun 24, 2022

View reviewed changes

Revert "fixing"

299c8e2

This reverts commit e07b0a0.

Madhav2310 added 5 commits June 26, 2022 02:18

lower added

644fc68

capitalize, upper, lower method functions added to builtin.py and int…

7726839

…egration tests made

expt

e80ba53

python functions code

22de62b

Add compile time upper, lower and capitalize in python_comptime_eval.h

3c8fa4a

Merge branch 'main' into madhav6

f980599

czgdp1807 reviewed Jul 8, 2022

View reviewed changes

integration_tests/test_builtin_lower.py Outdated Show resolved Hide resolved

Madhav2310 added 2 commits July 8, 2022 11:20

fixing

762a738

Merge branch 'madhav6' of https://github.com/Madhav2310/lpython into …

b537566

…madhav6

czgdp1807 requested changes Jul 8, 2022

View reviewed changes

Madhav2310 and others added 3 commits July 8, 2022 11:23

Update python_comptime_eval.h

42b7068

Update integration_tests/test_builtin_upper.py

560742a

Co-authored-by: Gagandeep Singh <gdp.1807@gmail.com>

Update integration_tests/test_builtin_lower.py

33e9e78

Co-authored-by: Gagandeep Singh <gdp.1807@gmail.com>

Madhav2310 added 2 commits July 8, 2022 11:52

suggested changes implemented

17838a0

Merge branch 'madhav6' of https://github.com/Madhav2310/lpython into …

6bcf65e

…madhav6

namannimmo10 reviewed Jul 8, 2022

View reviewed changes

src/lpython/semantics/python_comptime_eval.h Show resolved Hide resolved

Madhav2310 added 2 commits July 9, 2022 00:04

suggested changes implemented

f3ffcf3

fixing

eece0e5

certik mentioned this pull request Jul 11, 2022

Make int() work for non-constant strings #754

Merged

certik reviewed Jul 11, 2022

View reviewed changes

src/lpython/semantics/python_comptime_eval.h Outdated Show resolved Hide resolved

certik reviewed Jul 11, 2022

View reviewed changes

src/lpython/semantics/python_comptime_eval.h Outdated Show resolved Hide resolved

certik reviewed Jul 11, 2022

View reviewed changes

src/lpython/semantics/python_comptime_eval.h Outdated Show resolved Hide resolved

Madhav2310 added 2 commits July 12, 2022 19:52

fixing

48c5502

fixing 2.0

bdda9be

czgdp1807 added the could close Issues/PRs which can be closed. label Oct 5, 2022

certik marked this pull request as draft October 5, 2022 11:38

czgdp1807 added please take over PRs which can be taken over by other contributors and removed could close Issues/PRs which can be closed. labels Oct 5, 2022

Smit-create self-assigned this Nov 9, 2022

capitalize and upper functions (String module) #669

Are you sure you want to change the base?

capitalize and upper functions (String module) #669

Uh oh!

Conversation

Madhav2310 commented Jun 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Madhav2310 commented Jun 21, 2022

Uh oh!

certik commented Jun 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Madhav2310 commented Jun 23, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

czgdp1807 commented Jun 24, 2022

Uh oh!

namannimmo10 commented Jun 24, 2022

Uh oh!

namannimmo10 commented Jun 24, 2022

Uh oh!

Madhav2310 commented Jun 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Madhav2310 commented Jul 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

certik commented Jul 11, 2022

Uh oh!

Madhav2310 commented Jul 12, 2022

Uh oh!

Madhav2310 commented Jul 12, 2022

Uh oh!

certik commented Oct 5, 2022

Uh oh!

certik commented Oct 5, 2022

Uh oh!

czgdp1807 commented Oct 5, 2022

Uh oh!

Smit-create commented Nov 9, 2022

Uh oh!

Harsh-Upadhayay commented Dec 19, 2022

Uh oh!

Uh oh!

Madhav2310 commented Jun 21, 2022 •

edited

Loading

certik commented Jun 21, 2022 •

edited

Loading

Madhav2310 commented Jun 25, 2022 •

edited

Loading