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

Update python-can to 4.3.x for Python 3.12 and later #6557

Closed
wants to merge 1 commit into from

Conversation

nelsongraca
Copy link

This PR is a follow up on #6550

In Python 3.12 setuptools is no longer installed by default on virtual environment creation (see https://docs.python.org/3/whatsnew/3.12.html and python/cpython#95299)
This is breaking Klipper when connecting through CAN due to missing dependencies.

Latest python-can does not depend on setuptools, a conditional requirement was added for Python 3.12 and later, the same way it is currently done with greenlet.

I have tested on my setup and no issues, CAN connects and I can print.

@flowerysong
Copy link
Contributor

python-can 4.3 supports Python 3.8 and later, not just 3.12.

@nelsongraca
Copy link
Author

@flowerysong I just want to fix it from the version where it broke forward (3.12), there may be a side effect I haven't experienced and the less margin for error the better.

@flowerysong
Copy link
Contributor

If you can't do sufficient testing to be sure that this won't break things, it shouldn't be listed as a dependency for any version. If you can, it should be used as widely as possible.

@nelsongraca
Copy link
Author

So keep CAN completely broken on Python 3.12 and later?

As mentioned before, I did not find any issues but I'm not going to test every other Python version, there may be any sort of configuration that I'm not aware and break.

@nelsongraca
Copy link
Author

Any chance this will ever be merged? or will be left like many other PRs becoming stale

@nelsongraca
Copy link
Author

@KevinOConnor I'm not fond of tagging you directly but I'd like your take on this, if anyone installs Klipper with a new Python 3.12 venv and is using CAN it will be broken unless the user manually updates or installs a dependency, IMO being something that breaks a specific area of Klipper (CAN) makes it a little urgent.

@KevinOConnor
Copy link
Collaborator

Thanks. I'm leery of adding conditional requirements, as it increases maintenance costs (when troubleshooting what versions users are running become important as users run different software). If there is no version of python-can that runs on all python versions of interest, is there a version of setuptools that runs on all python versions of interest?

-Kevin

@KevinOConnor KevinOConnor added the pending feedback Topic is pending feedback from submitter label Apr 20, 2024
@nelsongraca
Copy link
Author

Hey Kevin, thank you for the feedback.

That was more the route I was taking with the previous PR #6550

I believe we can just add setuptools, I would do it conditionally for Python 3.12 or later since previous versions it's installed by default on venv creation, but there might be no harm if done on every version as it's am effective dependency because of python-can.

@KevinOConnor
Copy link
Collaborator

Okay, thanks. I think it would be preferable if we can find a version of setuptools that works everywhere.

-Kevin

Copy link

It looks like this GitHub Pull Request has become inactive. If there are any further updates, you can add a comment here or open a new ticket.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

@github-actions github-actions bot added the inactive Not currently being worked on label May 19, 2024
@github-actions github-actions bot closed this May 19, 2024
@nelsongraca nelsongraca deleted the update-python-can branch November 3, 2024 12:57
@KevinOConnor
Copy link
Collaborator

Thanks. I did commit a similar change (commit 01b0e98).

-Kevin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive Not currently being worked on pending feedback Topic is pending feedback from submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants