-
Notifications
You must be signed in to change notification settings - Fork 197
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
build: Modernize, use setuptools 69 #428
build: Modernize, use setuptools 69 #428
Conversation
This should solve the "Cannot find implementation or library stub" errors from mypy if installed properly. Since the code already has internal type hints, adding the marker is sufficient for mypy to use them.
The setuptools>=69 dependency includes a fix to automatically include the py.typed marker in the distribution, thus making type hints usage much easier. The newer version however fails to build when the basic project metadata is missing in pyproject.toml. Thus the settings from setup.cfg are migrated entirely, upgrading to setuptools_scm >= 8 in the process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically I'm not a collaborator for this project, but this change looks good.
@christiansandberg I would like your opinion on this, do you see any risk for larger breakage? |
This would be nice to get into an upcoming release, which we should make pretty soon to avoid more trouble like #455. OK @christiansandberg? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the absence. Go ahead and merge!
[project] | ||
name = "canopen" | ||
authors = [ | ||
{name = "Christian Sandberg", email = "[email protected]"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to add more people here. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acolomb It is ok :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I counted up the PRs that go into this release, and a large fraction of them are mine. Please feel free to use my name and email ([email protected]) in any release notes.
"test", | ||
] | ||
filterwarnings = [ | ||
"ignore::DeprecationWarning", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be useful to see anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe so. I suggest we leave it be until after the release. The only warnings I get currently are for datetime.utcnow()
and the bustype
argument to can.interface.Bus
.
Based on stats from GitHub, master branch as of today.
Migrate
setup.cfg
topyproject.toml
for newer setuptools compat and create py.typed marker for automatic type checker discovery.The
py.typed
marker should solve the "Cannot find implementation or library stub" errors from mypy if installed properly. Since the code already has internal type hints, adding the marker is sufficient for mypy to use them.The
setuptools>=69
dependency includes a fix to automatically include thepy.typed
marker in the distribution, thus making type hints usage much easier. The newer version however fails to build when the basic project metadata is missing inpyproject.toml
. Thus the settings fromsetup.cfg
are migrated entirely, upgrading tosetuptools_scm >= 8
inthe process.
This might need a requirement bump to Python >= 3.8 as well, since that's what the new setuptools version requires.