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

Add type hints to 'parsing/' package #939

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lavovaLampa
Copy link

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

umarcor commented Jun 25, 2023

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

lavovaLampa commented Jun 28, 2023

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

@umarcor
Copy link
Member

umarcor commented Jul 2, 2023

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

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