Skip to content

Support input in python#21

Open
hadisfr wants to merge 2 commits into
iradicek:masterfrom
hadisfr:support-input-python
Open

Support input in python#21
hadisfr wants to merge 2 commits into
iradicek:masterfrom
hadisfr:support-input-python

Conversation

@hadisfr

@hadisfr hadisfr commented Jul 17, 2020

Copy link
Copy Markdown
Contributor

add support of input() and some built-in functions to python interpreter and make it possible to get data from stdin in python

@hadisfr hadisfr force-pushed the support-input-python branch from 200d7ea to 93c0782 Compare July 17, 2020 21:25
@iradicek

Copy link
Copy Markdown
Owner

Sorry, for not looking at this and other PRs sooner, I somewhat missed them.

However, if it's not a problem I'd suggest also adding some tests for the new PR functionality.

Comment thread clara/py_parser.py
return Const('"{}"'.format(node.s), line=node.lineno)

def visit_NameConstant(self, node):
return Const('"{}"'.format(node.s), line=node.lineno)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Won't this create a string constant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let me check later. Maybe it should return a boolean value instead of string. 🤔

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think that NameConstant can be: "True", "False" or "None". So, I think that node.s should be passed to Const as a first argument instead of '"{}"'.format(node.s). However, I would add the very minimal tests to check this.

@hadisfr

hadisfr commented Sep 15, 2020

Copy link
Copy Markdown
Contributor Author

However, if it's not a problem I'd suggest also adding some tests for the new PR functionality.

OK. I'll try to add some tests only for input functionality, later.

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