parsing(c_parser): Added parsing code to handle the assignment case where character is assigned to integer #18925

smitgajjar · 2020-03-21T23:13:38Z

References to other Issues or PRs

Brief description of what is fixed or changed

Added the case where character is assigned to an integer

Added code in the function transform_character_literal() to return appropriate string
Handled the AST clang nodes in transform_var_decl()
Added tests for the same.

Other comments

Consider the following case :

In [1]: from sympy.parsing.sym_expr import SymPyExpression                      

In [2]: src = """ 
   ...: int a='1', b='d'; 
   ...: """                                                                     

In [3]: a=SymPyExpression(src, 'c')                                             

In [4]: a.return_expr()                                                         
Out[4]: 
[Declaration(Variable(a, type=integer, value=49)), Declaration(Variable(b, typ
e=integer, value=100))]

Before: Raised NotImplementedError
Now: Assigns correct ascii value to the integer value as expected in C and successfully gets parsed to sympy expression as shown above.

Release Notes

parsing
- Added the assignment case where character is assigned to an integer in C parser

sympy-bot · 2020-03-21T23:13:39Z

✅

Hi, I am the SymPy bot (v158). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

parsing
- Added the assignment case where character is assigned to an integer in C parser (#18925 by @smitgajjar)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
Added the case where character is assigned to an integer
- Added code in the function transform_character_literal() to return appropriate string
- Handled the AST clang nodes in transform_var_decl()
-  Added tests for the same.

#### Other comments
Consider the following case :
<pre>
In [1]: from sympy.parsing.sym_expr import SymPyExpression                      

In [2]: src = """ 
   ...: int a='1', b='d'; 
   ...: """                                                                     

In [3]: a=SymPyExpression(src, 'c')                                             

In [4]: a.return_expr()                                                         
Out[4]: 
[Declaration(Variable(a, type=integer, value=49)), Declaration(Variable(b, typ
e=integer, value=100))]

</pre>

Before: Raised NotImplementedError
Now: Assigns correct ascii value to the integer value as expected in C and successfully gets parsed to sympy expression as shown above.

#### Release Notes


<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
- parsing
    - Added the assignment case where character is assigned to an integer in C parser
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

codecov · 2020-03-22T01:56:28Z

Codecov Report

Merging #18925 into master will increase coverage by 0.084%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##           master    #18925       +/-   ##
============================================
+ Coverage   75.65%   75.734%   +0.084%     
============================================
  Files         647       647               
  Lines      168528    168564       +36     
  Branches    39710     39722       +12     
============================================
+ Hits       127492    127661      +169     
+ Misses      35476     35348      -128     
+ Partials     5560      5555        -5

Smit-create · 2020-03-22T09:29:46Z

sympy/parsing/c/c_parser.py

+
+            val : value


I think this should be

val : str Its description

Thanks for suggestion! I will change it as requested.

Sc0rpi0n101

I've left a few comments.
Otherwise, it looks good.

Sc0rpi0n101 · 2020-03-23T18:15:19Z

sympy/parsing/c/c_parser.py

-            #return val
-            pass
+               value = node.literal
+            return str(value[1])


why does this have to be value[1]?
Wouldn't value have the character literal that can directly be returned?

Actually, node.literal returns a str having value '<char>', whereas we only want <char>. So, it is a sort of workaround.

Sc0rpi0n101 · 2020-03-23T18:20:02Z

sympy/parsing/c/c_parser.py

@@ -256,6 +256,16 @@ def transform_var_decl(self, node):
                    ).as_Declaration(
                        value = val
                    )
+                elif (child.kind == cin.CursorKind.CHARACTER_LITERAL


Can you add a comment here describing when it will be called?

Sure, will add it in the next commit.

Sc0rpi0n101 · 2020-03-23T18:58:47Z

I think the rest looks good.

Can you also squash your commits into one with a description of all the changes?

smitgajjar · 2020-03-23T19:33:03Z

I guess, this longer commit message looks weird! Anyways, did the squash! :)

smichr · 2020-03-23T22:28:13Z

commit message looks weird

If you add a blank line between title of commit and commit message it will look better:

title blah blah

comments

smitgajjar · 2020-03-23T22:42:53Z

Alright, thanks for the help, I will do it now!

C parser code could not handle the situation, where a variable which is declared as integer has been assigned with a character literal. For this, the function transform_character_literal() has been completed so as to handle character literals to be used in this case. Then, in transform_var_decl() function, after getting the node value from previous function, this case is handled in a separate elif block along with the comment giving its explanation. Also, corresponding test is added.

smitgajjar changed the title ~~handled the case where character is assigned to integer~~ parsing(c_parser): Added parsing code to handle the assignment case where character is assigned to integer Mar 21, 2020

Smit-create reviewed Mar 22, 2020

View reviewed changes

Abdullahjavednesar added the parsing label Mar 22, 2020

Sc0rpi0n101 reviewed Mar 23, 2020

View reviewed changes

smitgajjar force-pushed the assign_char_to_int branch from 406c3e1 to 4e1b752 Compare March 23, 2020 19:23

smitgajjar force-pushed the assign_char_to_int branch from 4e1b752 to 81e5e04 Compare March 23, 2020 23:02

Sc0rpi0n101 merged commit ce1e53f into sympy:master Mar 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parsing(c_parser): Added parsing code to handle the assignment case where character is assigned to integer #18925

parsing(c_parser): Added parsing code to handle the assignment case where character is assigned to integer #18925

smitgajjar commented Mar 21, 2020 •

edited

sympy-bot commented Mar 21, 2020 •

edited

codecov bot commented Mar 22, 2020 •

edited

Smit-create Mar 22, 2020

smitgajjar Mar 22, 2020

Sc0rpi0n101 left a comment

Sc0rpi0n101 Mar 23, 2020

smitgajjar Mar 23, 2020 •

edited

Sc0rpi0n101 Mar 23, 2020

Sc0rpi0n101 Mar 23, 2020

smitgajjar Mar 23, 2020

Sc0rpi0n101 commented Mar 23, 2020

smitgajjar commented Mar 23, 2020

smichr commented Mar 23, 2020

smitgajjar commented Mar 23, 2020

parsing(c_parser): Added parsing code to handle the assignment case where character is assigned to integer #18925

parsing(c_parser): Added parsing code to handle the assignment case where character is assigned to integer #18925

Conversation

smitgajjar commented Mar 21, 2020 • edited

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

sympy-bot commented Mar 21, 2020 • edited

codecov bot commented Mar 22, 2020 • edited

Codecov Report

Smit-create Mar 22, 2020

Choose a reason for hiding this comment

smitgajjar Mar 22, 2020

Choose a reason for hiding this comment

Sc0rpi0n101 left a comment

Choose a reason for hiding this comment

Sc0rpi0n101 Mar 23, 2020

Choose a reason for hiding this comment

smitgajjar Mar 23, 2020 • edited

Choose a reason for hiding this comment

Sc0rpi0n101 Mar 23, 2020

Choose a reason for hiding this comment

Sc0rpi0n101 Mar 23, 2020

Choose a reason for hiding this comment

smitgajjar Mar 23, 2020

Choose a reason for hiding this comment

Sc0rpi0n101 commented Mar 23, 2020

smitgajjar commented Mar 23, 2020

smichr commented Mar 23, 2020

smitgajjar commented Mar 23, 2020

smitgajjar commented Mar 21, 2020 •

edited

sympy-bot commented Mar 21, 2020 •

edited

codecov bot commented Mar 22, 2020 •

edited

smitgajjar Mar 23, 2020 •

edited