Add dict comprehension support

[Zehen-249]

Pull Request description

Add Dictionary Comprehension Support

Solve #198

How to test these changes

Check Tutorials folder there is a notebook file for dict_comprehension

Pull Request checklists

Note:

• Share updated images of the graph representation of the new ASTx node
  proposed in this PR, in both image and ASCII formats. For more
  information, check this Google Colab notebook:
  https://colab.research.google.com/drive/1xXwHmOMkJKFSmhRvn4WYfSAsdDzMnawf?usp=sharing

This PR is a:

• bug-fix
• new feature
• maintenance

About this PR:

• it includes tests.
• the tests are executed on CI.
• the tests generate log file(s) (path).
• pre-commit hooks were executed locally.
• this PR requires a project documentation update.

Author's checklist:

• I have reviewed the changes and it contains no misspelling.
• The code is well commented, especially in the parts that contain more
  complexity.
• New and old tests passed locally.

Additional information

Dict Comprehension object

String Representation

{Variable[x]: BinaryOp[*](Variable[x],Variable[x]) for Variable[x] in Variable[range(10)] if BinaryOp[>](Variable[x],LiteralInt32(5)) if BinaryOp[<](Variable[x],LiteralInt32(7))}

Image Representation

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved .

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[xmnlab]

@Zehen-249 , thanks for working on that. please take a look into the errors in the CI pls

[Zehen-249]

@Zehen-249 , thanks for working on that. please take a look into the errors in the CI pls

All checks passed locally.

[Zehen-249]

Hey @xmnlab can you review this PR now

[yuvi-mittal]

Heyy @Zehen-249 , Can you please remove the file docs/tutorials/dict_comprehension.ipynb it might create merge conflict later , it might be because you are force pushing it . Also would you mind adding Ascii representation too

[yuvi-mittal]

Add unique id to the key here for better representation.

[Zehen-249]

Added unique id for the key.
I am not sure about the ASCII representation, i could not find any class implementing this ?
Could you please guide me to find reference for ASCII representation of ASTx Node.

[yuvi-mittal]

@Zehen-249 , you can call your test function(suppose test_if_stmt() ) in your test_flows.py , and add a print statement ( ( print(repr(if_stmt) ) in your test function, so when you run your file , it will print ascii representation as output . If you still have doubt you can check out some prev PRs there are screenshots available.

[Zehen-249]

@Zehen-249 , you can call your test function(suppose test_if_stmt() ) in your test_flows.py , and add a print statement ( ( print(repr(if_stmt) ) in your test function, so when you run your file , it will print ascii representation as output . If you still have doubt you can check out some prev PRs there are screenshots available.

if I want to call repr function I must have defined the repr function, which I haven't. Also in the ASTx site it is mention how to implement str and get_structs method but nothing for repr method. I ran grep on the src code of astx to find implementation of repr for classes and no class implements this method. I also looked into some previously merged PRs no one has did anything like this .

[Zehen-249]

If you were referring to this ?

[xmnlab]

instead of using transpiler.visit, please use translate (e.g. https://github.com/arxlang/astx/pull/232/files#diff-213fa69c2a6fc65240961486159b29cc98cffaab2968b96afbae37a47abc4624R52)

it uses the translate.visit + a function to check if the syntax is correct

[xmnlab]

hi @Zehen-249

please consider the following structure from python:

"Module(body=[Expr(value=DictComp(key=Name(id='x', ctx=Load()), value=Name(id='x', ctx=Load()), generators=[comprehension(target=Name(id='x', ctx=Store()), iter=Call(func=Name(id='range', ctx=Load()), args=[Constant(value=10)], keywords=[]), ifs=[], is_async=0)]))], type_ignores=[])"
>>> 

as you can see it just has the attributes: key, value, and generators.
so please update the class attributes and the arguments in __init__

in the attribute generators it will be a list of Comprehension.

let me know if you have any questions.

[Zehen-249]

if generator is to be used the python syntax will be like
{x : x * x for x in ( ( x + x ) for x in [ 1, 2, 3, 4, 5, 6 ] if ( ( x % 2 ) == 0 ) ) }
which has generator ( ( x + x ) for x in [ 1, 2, 3, 4, 5, 6 ] if ( ( x % 2 ) == 0 ) ) but AStx does not have generator expr class implemented yet and using that will be more convincing i guess ???

[Zehen-249]

If I am on right understanding ?

[xmnlab]

hey @Zehen-249 , you should use the Comprehension class, instead of the Generator.

actually Generator should use comprehension as well. https://github.com/arxlang/astx/pull/232/files#diff-3ff6d1ecb86cdf40792ce4c94a5fc72d948eb8d389fb79dfce984b62819b9b39R748-R750

"Module(body=[Expr(value=GeneratorExp(elt=Name(id='x', ctx=Load()), generators=[comprehension(target=Name(id='x', ctx=Store()), iter=Call(func=Name(id='range', ctx=Load()), args=[Constant(value=10)], keywords=[]), ifs=[], is_async=0)]))], type_ignores=[])"
>>> 

could you open a new PR to fix that pls?

[Zehen-249]

image Representaion

ASCIII Representation

String representation

DictComprehension[key=Variable[x], value=BinaryOp[+](Variable[x],Variable[x]), target=Variable[x], iterable=Variable[range(10)],  conditions=['BinaryOp[>](Variable[x],LiteralInt32(5))', 'BinaryOp[<](Variable[x],LiteralInt32(7))'], is_async=False]

Trasnpiled Python Code

{x: (x + x) for x in range(10) if (x > 5) if (x < 7)}

[Zehen-249]

hey @xmnlab,
I have made the required changes in Dict Comprehension and also have Opened a new PR for Generator expr class update

[xmnlab]

@Zehen-249 I just merged this PR: #245

let me know if it clear now. sorry for the confusion.

[Zehen-249]

@Zehen-249 I just merged this PR: #245

let me know if it clear now. sorry for the confusion.

Hey @xmnlab in the dictcompehension
Key value pair targets are also used

{key:value for key, value in [(a,1),(b,2),(c,3)]}

How one can set the target variable to be key, value. One can use LiteralString maybe but thats not logically correct??

[Zehen-249]

@Zehen-249 I just merged this PR: #245
let me know if it clear now. sorry for the confusion.

Hey @xmnlab in the dictcompehension Key value pair targets are also used

{key:value for key, value in [(a,1),(b,2),(c,3)]}

How one can set the target variable to be key, value. One can use LiteralString maybe but thats not logically correct??

I tried using LiteralTuple for target but the problem is the elements of the LiteralTuple must be Literal types and so i can not use Variable or identifiers

[xmnlab]

hi @Zehen-249 ,

I always use python ast as a reference:

>>> import ast
>>> ast.dump(ast.parse("{key:value for key, value in [(a,1),(b,2),(c,3)]}"))
"Module(body=[Expr(value=DictComp(key=Name(id='key', ctx=Load()), value=Name(id='value', ctx=Load()), generators=[comprehension(target=Tuple(elts=[Name(id='key', ctx=Store()), Name(id='value', ctx=Store())], ctx=Store()), iter=List(elts=[Tuple(elts=[Name(id='a', ctx=Load()), Constant(value=1)], ctx=Load()), Tuple(elts=[Name(id='b', ctx=Load()), Constant(value=2)], ctx=Load()), Tuple(elts=[Name(id='c', ctx=Load()), Constant(value=3)], ctx=Load())], ctx=Load()), ifs=[], is_async=0)]))], type_ignores=[])"

[xmnlab]

let me know if that makes sense, pls

[Zehen-249]

hi @Zehen-249 ,

I always use python ast as a reference:

>>> import ast
>>> ast.dump(ast.parse("{key:value for key, value in [(a,1),(b,2),(c,3)]}"))
"Module(body=[Expr(value=DictComp(key=Name(id='key', ctx=Load()), value=Name(id='value', ctx=Load()), generators=[comprehension(target=Tuple(elts=[Name(id='key', ctx=Store()), Name(id='value', ctx=Store())], ctx=Store()), iter=List(elts=[Tuple(elts=[Name(id='a', ctx=Load()), Constant(value=1)], ctx=Load()), Tuple(elts=[Name(id='b', ctx=Load()), Constant(value=2)], ctx=Load()), Tuple(elts=[Name(id='c', ctx=Load()), Constant(value=3)], ctx=Load())], ctx=Load()), ifs=[], is_async=0)]))], type_ignores=[])"

Yeah I did this, but the same can't be replicated in ASTx bcoz, x,y is taken as tuple of Name type element and in ast it is allowed for Tuple node to store Name types but in ASTx implementation we only only have LiteralTuple in which we can only store Literal types, and Variable (which is counter for Name in ast i suppose) is not a Literal Type

Both Literal and Variable inherit from DataTypeOb

[Zehen-249]

hi @Zehen-249 ,

I always use python ast as a reference:

>>> import ast
>>> ast.dump(ast.parse("{key:value for key, value in [(a,1),(b,2),(c,3)]}"))
"Module(body=[Expr(value=DictComp(key=Name(id='key', ctx=Load()), value=Name(id='value', ctx=Load()), generators=[comprehension(target=Tuple(elts=[Name(id='key', ctx=Store()), Name(id='value', ctx=Store())], ctx=Store()), iter=List(elts=[Tuple(elts=[Name(id='a', ctx=Load()), Constant(value=1)], ctx=Load()), Tuple(elts=[Name(id='b', ctx=Load()), Constant(value=2)], ctx=Load()), Tuple(elts=[Name(id='c', ctx=Load()), Constant(value=3)], ctx=Load())], ctx=Load()), ifs=[], is_async=0)]))], type_ignores=[])"

Yeah I did this, but the same can't be replicated in ASTx bcoz, x,y is taken as tuple of Name type element and in ast it is allowed for Tuple node to store Name types but in ASTx implementation we only only have LiteralTuple in which we can only store Literal types, and Variable (which is counter for Name in ast i suppose) is not a Literal Type

Both Literal and Variable inherit from DataTypeOb

No, variables in Python are not subclasses of Literal. The Literal type in typing is used to annotate functions or values that are restricted to specific constant values — not to represent actual variable types.

[xmnlab]

Sorry I missed your point. That is right, the current implementation is not correct. Good catch.

If you want to work on that, please go ahead. Otherwise I will fix that soon.

For now, probably it should be Expr. It seems that other collection classes are wrong.

[Zehen-249]

Sorry I missed your point. That is right, the current implementation is not correct. Good catch.

If you want to work on that, please go ahead. Otherwise I will fix that soon.

For now, probably it should be Expr. It seems that other collection classes are wrong.

Hey @xmnlab, I would be happy to contribute. For now I believe changing the datatypes from Literals to DataTypeObs would do the job. I will do some more research and will raise an issue.

[xmnlab]

thanks @Zehen-249 !

[github-actions]

This pull request has been marked as stale because it has been
inactive for more than 5 days. Please update this pull request
or it will be automatically closed in 5 days.

[github-actions]

This pull request has been marked as stale because it has been
inactive for more than 5 days. Please update this pull request
or it will be automatically closed in 5 days.