Skip to content

Conversation

@lavovaLampa
Copy link

Move project metadata from setup.py to pyproject.toml. This change should make project more future-proof as the Python ecosystem is moving to pyproject.toml format. Moreover, it fixes problems with dependency definitions (as they are defined statically).

"License :: OSI Approved :: Mozilla Public License 2.0 (MPL 2.0)",
"Natural Language :: English",
"Intended Audience :: Developers",
"Programming Language :: Python :: 3.7",
Copy link

Choose a reason for hiding this comment

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

Just note if support should be 3.6 include it here as well

]

[tool.setuptools.dynamic]
version = {attr = "vunit.about.version"}
Copy link

Choose a reason for hiding this comment

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

This version will be contradicting the one from setuptools-scm. Witch one will be used?

A little bit unsure about how the versioning is managed or intended to work for this project 😕. So this comment might just be me not geting something 😄

[tool.setuptools.dynamic]
version = {attr = "vunit.about.version"}

[tool.black]
Copy link

Choose a reason for hiding this comment

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

Sorry for this one just noticed going through the file. It's of topic but would be a nice fix to get in.

force-exclude or extended-exclude could be added here to have global black exclude list and not one just for tox
See black docs

force-exclude seems to be the only one that actually takes effect

@@ -0,0 +1,3 @@
recursive-include vunit/ *.tcl
Copy link

Choose a reason for hiding this comment

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

This could be done inside the pyproject.toml as well 😄 I assume the goal with this is to have all the config in one place

[tool.setuptools.package-data]
vunit = [
   # Script and wave files
   "**/*.do",
   "**/*.tcl",
   # Verilog
   "**/*.v",
   "**/*.sv",
   "**/*.svh",
   # VHDL
   "**/*.vhd",
   "**/*.vhdl",
]

Is the .do files intentionally skipped?

pyproject.toml Outdated
@@ -1,18 +1,64 @@
[build-system]
requires = [
Copy link

Choose a reason for hiding this comment

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

If the move towards pyproject.toml is made could it be done inline with the current recommendations from setuptools-scm

Minimal just the scm stuff

[build-system]
requires = ["setuptools>=45", "setuptools_scm[toml]>=6.2"] # this would be version bumps
...
[project]
dynamic = ["version"]
...
[tool.setuptools_scm]  # this is required to activate the scm functionality
...

As there might be some changes needed due to to the move to pyproject.toml it would be nice bumping the versions to the recommended once 😄

license = { text = "Mozilla Public License 2.0 (MPL 2.0)" }
description = "VUnit is an open source unit testing framework for VHDL/SystemVerilog."
readme = "README.md"
requires-python = ">=3.7"
Copy link

Choose a reason for hiding this comment

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

This should be 3.6 right? or is it dropped due to the EOL? In that case time to drop 3.7 as well 🥲

)

if not osvvm_is_installed():
if len(glob(str(Path(__file__).parent / "vunit" / "vhdl" / "osvvm" / "*.vhd"))) == 0:
Copy link

Choose a reason for hiding this comment

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

As Path is already build we can just use glob method of the pathlib.Path object. rglob also exists if needed

This returns a iterator that we can call next on with a default value of None to get the len(...) == 0 behaviour.

OSVVM_PATH = Path(__file__).with_name("vunit") / "vhdl" / "osvvm"
if next(OSVVM_PATH.glob("*.vhd"), None) is None:
    warning(...)

@eine eine added this to the v5.0.0 milestone Mar 11, 2024
@eine eine modified the milestones: v5.0.0, v5.1.0 Mar 11, 2024
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