Add PLY Parser #84
Add PLY Parser #84
Conversation
There was a problem hiding this comment.
Thanks for the great work. I think the test code sets up an example for others.
One problem is mandatory to be fixed before we can merge this PR, that is you need to install qelib1.inc, non-py files are automatically excluded when using pip install . I also commented on the details.
BTW, please also include the license header for new files.
Other comments are just some general suggestions and you can igore them.
Besides, there're some common practice we can try to follow
- Split large PR into smaller ones to maker life easier for reviewers.
- For such a complete feature, I think a document in yuque is needed.
- We can set up an agreement on which functions need detailed comment, i.e.,
Args,Returns, etc. Typically APIs exposed to users need these instructions. For other small functions or internal member functions we don't need detailed comments.
| def input(self, data): | ||
| self.data = data | ||
| self.lexer.input(data) | ||
| # read qelib1.inc |
There was a problem hiding this comment.
The file qelib1.inc needs to be installed through pip install . You can refer to this link to install non-py files: https://setuptools.pypa.io/en/latest/userguide/datafiles.html
And this is the root cause of ut failure: https://github.com/ScQ-Cloud/pyquafu/actions/runs/6186274257/job/16793521132
You can also enable github actions in your forked repository, then you can test whether this issue has been fixed
| break | ||
| print(tok, file=print_file) | ||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
Suggest not to include these test codes in source file
| ] | ||
|
|
||
|
|
||
| # 从最底层向上归约写 |
There was a problem hiding this comment.
Better to avoid chinese comments as it is opensource to the entire community
| self.is_qubit = is_qubit | ||
| self.qargs = [] | ||
| self.cargs = [] | ||
| self.instructions = [] |
There was a problem hiding this comment.
Needs a blank line. Perhaps we can use black in the future.
| self.filename = filename | ||
| self.num = 0 | ||
|
|
||
| class IndexedId(Node): |
There was a problem hiding this comment.
Not sure whether it would be better to use dataclass for these Nodes.
Add PLY Parser and test for it.