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

Refactor Parser/pgen and add documentation and explanations #15373

Merged
merged 4 commits into from
Aug 22, 2019

Conversation

pablogsal
Copy link
Member

To improve the readability and maintainability of the parser
generator perform the following transformations:

* Separate the metagrammar parser in its own class to simplify
  the parser generator logic.

* Create separate classes for DFAs and NFAs and move methods that
  act exclusively on them from the parser generator to these
  classes.

* Add docstrings and comment docummenting the process to go from
  the grammar file into NFAs and then DFAs. Detail some of the
  algorithms and give some background explanations of some concepts
  that will helps readers not familiar with the parser generation
  process.

* Select more descriptive names for some variables and variables.

* PEP8 formatting and quote-style homogenization.

The output of the parser generator remains the same (Include/graminit.h
and Python/graminit.c remain untached by running the new parser generator).

@pablogsal
Copy link
Member Author

pablogsal commented Aug 21, 2019

@willingc After a couple of days of work I finished this PR that is doing some refactors to modernize the code and make it less coupled and more maintainable (not very important - the generated parser is untouched) but also is adding multiple docstrings and comments explaining how everything works, some background concepts and clarifications that I think they are very important for any reader not very familiar with LL(1) parsing.

If you have some time, could you review these docstrings and comments to make sure is clear enough and there is nothing you think is missing? Although I am happy with the current form your input will surely make a difference :)

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

@pablogsal This is mergeable as is or with the suggested wording changes. Great work.

Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
Parser/pgen/automata.py Outdated Show resolved Hide resolved
To improve the readability and maintainability of the parser
generator perform the following transformations:

    * Separate the metagrammar parser in its own class to simplify
      the parser generator logic.

    * Create separate classes for DFAs and NFAs and move methods that
      act exclusively on them from the parser generator to these
      classes.

    * Add docstrings and comment documenting the process to go from
      the grammar file into NFAs and then DFAs. Detail some of the
      algorithms and give some background explanations of some concepts
      that will helps readers not familiar with the parser generation
      process.

    * Select more descriptive names for some variables and variables.

    * PEP8 formatting and quote-style homogenization.

The output of the parser generator remains the same (Include/graminit.h
and Python/graminit.c remain untouched by running the new parser generator).
pablogsal and others added 2 commits August 22, 2019 01:50
Co-Authored-By: Carol Willing <carolcode@willingconsulting.com>
@pablogsal
Copy link
Member Author

Thank you very much @willingc for the thorough review!

I have applied all your suggestions and rephrasing and the comments and docstrings read much clearer now. Thanks!

@pablogsal pablogsal merged commit 71876fa into python:master Aug 22, 2019
@pablogsal pablogsal deleted the the_pgen branch August 22, 2019 01:38
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
…-15373)

* Refactor Parser/pgen and add documentation and explanations

To improve the readability and maintainability of the parser
generator perform the following transformations:

    * Separate the metagrammar parser in its own class to simplify
      the parser generator logic.

    * Create separate classes for DFAs and NFAs and move methods that
      act exclusively on them from the parser generator to these
      classes.

    * Add docstrings and comment documenting the process to go from
      the grammar file into NFAs and then DFAs. Detail some of the
      algorithms and give some background explanations of some concepts
      that will helps readers not familiar with the parser generation
      process.

    * Select more descriptive names for some variables and variables.

    * PEP8 formatting and quote-style homogenization.

The output of the parser generator remains the same (Include/graminit.h
and Python/graminit.c remain untouched by running the new parser generator).
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
…-15373)

* Refactor Parser/pgen and add documentation and explanations

To improve the readability and maintainability of the parser
generator perform the following transformations:

    * Separate the metagrammar parser in its own class to simplify
      the parser generator logic.

    * Create separate classes for DFAs and NFAs and move methods that
      act exclusively on them from the parser generator to these
      classes.

    * Add docstrings and comment documenting the process to go from
      the grammar file into NFAs and then DFAs. Detail some of the
      algorithms and give some background explanations of some concepts
      that will helps readers not familiar with the parser generation
      process.

    * Select more descriptive names for some variables and variables.

    * PEP8 formatting and quote-style homogenization.

The output of the parser generator remains the same (Include/graminit.h
and Python/graminit.c remain untouched by running the new parser generator).
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
…-15373)

* Refactor Parser/pgen and add documentation and explanations

To improve the readability and maintainability of the parser
generator perform the following transformations:

    * Separate the metagrammar parser in its own class to simplify
      the parser generator logic.

    * Create separate classes for DFAs and NFAs and move methods that
      act exclusively on them from the parser generator to these
      classes.

    * Add docstrings and comment documenting the process to go from
      the grammar file into NFAs and then DFAs. Detail some of the
      algorithms and give some background explanations of some concepts
      that will helps readers not familiar with the parser generation
      process.

    * Select more descriptive names for some variables and variables.

    * PEP8 formatting and quote-style homogenization.

The output of the parser generator remains the same (Include/graminit.h
and Python/graminit.c remain untouched by running the new parser generator).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants