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

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

Merged
merged 16 commits into from
Jul 11, 2022

Conversation

MatthieuDartiailh
Copy link
Collaborator

No description provided.

@pablogsal
Copy link
Contributor

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

@MatthieuDartiailh
Copy link
Collaborator Author

Will do

@MatthieuDartiailh
Copy link
Collaborator Author

@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
Copy link
Contributor

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
Copy link
Collaborator Author

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
Copy link
Contributor

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
Copy link
Collaborator Author

Ping @pablogsal

@lysnikolaou
Copy link
Contributor

Looking as well.

@lysnikolaou
Copy link
Contributor

lysnikolaou commented May 25, 2022

@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
Copy link
Collaborator Author

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 MatthieuDartiailh marked this pull request as ready for review May 30, 2022 11:29
@MatthieuDartiailh
Copy link
Collaborator Author

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
Copy link
Collaborator Author

ping @pablogsal @lysnikolaou

@MatthieuDartiailh
Copy link
Collaborator Author

Friendly ping @pablogsal @lysnikolaou

@MatthieuDartiailh
Copy link
Collaborator Author

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

@pablogsal pablogsal merged commit 36854c6 into we-like-parsers:main Jul 11, 2022
@MatthieuDartiailh MatthieuDartiailh deleted the pep527 branch July 11, 2022 18:05
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.

3 participants