Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compile-time support for dict.values #2661

Merged
merged 10 commits into from
Apr 27, 2024

Conversation

kmr-srbh
Copy link
Contributor

@kmr-srbh kmr-srbh commented Apr 24, 2024

Working

from lpython import i32


print({1: "a"}.values())
print({"a": 1, "b": 2, "c": 3}.values())
print({1: [1, 2, 3], 2: [4, 5, 6], 3: [7, 8, 9]}.values())
print({(1, 2): "a", (3, 4): "b", (5, 6): "c"}.values())

k_1: list[list[i32]] = {
    "list1": [1, 2, 3],
    "list2": [4, 5, 6],
    "list3": [7, 8, 9],
}.values()
print(k_1)

k_2: list[str] = {(1, 2): "a", (3, 4): "b", (5, 6): "c"}.values()
print(k_2)

CPython

(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ python ./examples/example.py
dict_values(['a'])
dict_values([1, 2, 3])
dict_values([[1, 2, 3], [4, 5, 6], [7, 8, 9]])
dict_values(['a', 'b', 'c'])
dict_values([[1, 2, 3], [4, 5, 6], [7, 8, 9]])
dict_values(['a', 'b', 'c'])

LPython

(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
['a']
[1, 2, 3]
[[1, 2, 3], [4, 5, 6], [7, 8, 9]]
['a', 'b', 'c']
[[1, 2, 3], [4, 5, 6], [7, 8, 9]]
['a', 'b', 'c']

@Shaikh-Ubaid
Copy link
Collaborator

@kmr-srbh whenever something works with lpython, we also need it to work with cpython and produce a similar output. Can you post the output for the example in the PR description for cpython? Can you update your other PRs to show output for cpython? Thanks!

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as draft April 25, 2024 19:33
@kmr-srbh
Copy link
Contributor Author

Can you update your other PRs to show output for cpython?

I am posting the output. 👍

@Shaikh-Ubaid
Copy link
Collaborator

Shaikh-Ubaid commented Apr 26, 2024

% cat examples/expr2.py    
print({1: "a"}.values())
% python examples/expr2.py 
dict_values(['a'])
% lpython examples/expr2.py
['a']
% cat examples/expr2.py   
print({1: "a"}.values()[0])
% python examples/expr2.py 
Traceback (most recent call last):
  File "/Users/ubaid/Desktop/OpenSource/lpython/examples/expr2.py", line 1, in <module>
    print({1: "a"}.values()[0])
TypeError: 'dict_values' object is not subscriptable
% lpython examples/expr2.py
a

It seems technically a list and a dict_values are not the same.

@Shaikh-Ubaid
Copy link
Collaborator

Please mark as "Ready for review" when ready.

@Shaikh-Ubaid
Copy link
Collaborator

@certik do you have views on #2661 (comment)? This PR returns dict.values() as list. Similarly other PR #2660 returns dict.keys() as list. In cpython dict.keys() and dict.values() return as object dict_values and object dict_keys.

@kmr-srbh
Copy link
Contributor Author

@Shaikh-Ubaid I return a list for dict.keys and dict.values following #1881 (comment)

@Shaikh-Ubaid
Copy link
Collaborator

I return a list for dict.keys and dict.values following #1881 (comment)

Thanks for sharing. It makes sense.

@kmr-srbh kmr-srbh marked this pull request as ready for review April 27, 2024 11:42
@Shaikh-Ubaid
Copy link
Collaborator

After #2660 gets merged, there can be conflicts in this PR. Please resolve them after #2660 is merged. I will give it a final review then.

@kmr-srbh
Copy link
Contributor Author

@Shaikh-Ubaid the conflicts have been resolved.

@Shaikh-Ubaid Shaikh-Ubaid merged commit c63882a into lcompilers:main Apr 27, 2024
14 checks passed
assem2002 pushed a commit to assem2002/lpython that referenced this pull request Apr 28, 2024
* Implement `dict.values` for `DictConstant`

* Tests: Add tests

* Tests: Fix typing mistake

* Tests: Update references

* Uncomment check for modifying attribute

* Tests: Update references

* Delete tests/reference/asr-func_04-eef2656.stdout

* Tests: Move `print` before `assert`

* Tests: Update test
assem2002 pushed a commit to assem2002/lpython that referenced this pull request Apr 28, 2024
* Implement `dict.values` for `DictConstant`

* Tests: Add tests

* Tests: Fix typing mistake

* Tests: Update references

* Uncomment check for modifying attribute

* Tests: Update references

* Delete tests/reference/asr-func_04-eef2656.stdout

* Tests: Move `print` before `assert`

* Tests: Update test
@kmr-srbh kmr-srbh deleted the dict-constant-values branch May 2, 2024 13:58
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