search

we-like-parsers / pegen

PEG parser generator for Python
https://we-like-parsers.github.io/pegen/
MIT License
150 stars 32 forks source link

Maintenance: move most of the configuration to pyproject.toml #66

Closed MatthieuDartiailh closed 2 years ago

pablogsal commented 2 years ago

This is a great change. Ping me when is ready 👍

MatthieuDartiailh commented 2 years ago

Will do

MatthieuDartiailh commented 2 years ago

@pablogsal I am unsure how to handle the case of the start method on a parser. We have a bunch of utils making that assumption but the grammar does not force it to be so. As a consequence mypy rightfully complains and I am not sure how to appease it in a meaningful way.

pablogsal commented 2 years ago

I am afraid I am going to need more context here. Why porting the configuration to pyproject.toml causes mypy to complain about the parser start method?

MatthieuDartiailh commented 2 years ago

I do not know when but the CIs were disabled for Python 3.8 and 3.9 at one point causing some mypy errors to sneak in. You can check the Python 3.8 failure if you have time to do so.

pablogsal commented 2 years ago

You can check the Python 3.8 failure if you have time to do so.

I will try to do it this week, but I will probably be unable after PyConUS. Maybe @lysnikolaou or @isidentical could take a look here?

MatthieuDartiailh commented 2 years ago

Ping @pablogsal

lysnikolaou commented 2 years ago

Looking as well.

lysnikolaou commented 2 years ago

@pablogsal I am unsure how to handle the case of the start method on a parser. We have a bunch of utils making that assumption but the grammar does not force it to be so. As a consequence mypy rightfully complains and I am not sure how to appease it in a meaningful way.

I think the solution here might be making Parser an abstract base class and adding an abstract method start. We make the assumption that there is a start method (or rule) pretty much everywhere, so I can't see why we shouldn't do this.

MatthieuDartiailh commented 2 years ago

That would indeed work. I was unsure about taking this direction this it is not enforced by the grammar. I will update the patch when I get a moment (may take me a week or so).

MatthieuDartiailh commented 2 years ago

I went ahead with the ABC change and also enabled flake8. This is ready to go from my end but should probably be squashed given the stupid number of iterations it took me.

As a side question now that we do not have C anymore is there still value in keeping make ? I mostly develop on windows and being unable to run tox is a tad annoying.

MatthieuDartiailh commented 2 years ago

ping @pablogsal @lysnikolaou

MatthieuDartiailh commented 2 years ago

Friendly ping @pablogsal @lysnikolaou

MatthieuDartiailh commented 2 years ago

I know 3.11.0.b4 represents a lot of work but would have time to review this @pablogsal @lysnikolaou