Skip to content

Add type hints to 'parsing/' package#939

Open
lavovaLampa wants to merge 5 commits into
VUnit:masterfrom
lavovaLampa:typing
Open

Add type hints to 'parsing/' package#939
lavovaLampa wants to merge 5 commits into
VUnit:masterfrom
lavovaLampa:typing

Conversation

@lavovaLampa

Copy link
Copy Markdown

Initial batch of type hints. Also refactored token kinds into an Enum, as Python 3.6 is no longer supported.

Currently depends on some types provided by package 'typing-extensions', in case this new dependency is unwelcome, I can remove dependent type hints.

@umarcor

umarcor commented Jun 25, 2023

Copy link
Copy Markdown
Member

CI is failing because the required package is not installed. You can specify it in https://github.com/VUnit/vunit/blob/master/pyproject.toml#L64-L80, since all the tests are executed through tox.

@lavovaLampa

Copy link
Copy Markdown
Author

While setuptools is trying to determine dependencies, it has to run the setup.py script. It then tries to import other modules, running into an import from typing_extensions. AFAIK, setuptools builds packages in isolated environment, meaning this dependency is not present at the time of dependency resolution (makes sense).

@lavovaLampa

lavovaLampa commented Jun 28, 2023

Copy link
Copy Markdown
Author

Project metadata could be alternatively moved into pyproject.toml. In any case this should solve the issue of dependency definitions during build.

@umarcor

umarcor commented Jul 2, 2023

Copy link
Copy Markdown
Member

@lavovaLampa could you please make 7cfdaec a separated PR (which we can merge before this)?

@lavovaLampa

Copy link
Copy Markdown
Author

Opened #942

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.

3 participants