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

Remove imp module from kernel.py #1361

Closed
wants to merge 1 commit into from

Conversation

pnfox
Copy link

@pnfox pnfox commented Nov 25, 2024

I understand that ck is archived. However ck does not work with python3.12+ since the imp module has been removed.

This PR makes use of importlib instead of imp and allows ck to work with python3.12 and python3.13.

If this PR cannot be approved due to ck being archived, would it be possible to document this information patch somewhere? It would save time for others also looking into this.

Thanks
Patrick

@pnfox pnfox requested a review from a team as a code owner November 25, 2024 11:21
Copy link

MLCommons CLA bot:
Thank you very much for your submission, we really appreciate it. Before we can accept your contribution, we ask that you sign the MLCommons CLA (Apache 2). Please use this [Google form] (https://forms.gle/Ew1KkBVpyeJDuRw67) to initiate authorization. If you are from an MLCommons member organization, we will request that you be added to the CLA. If you are not from a member organization, we will email you a CLA to sign. For any questions, please contact support@mlcommons.org.
0 out of 1 committers have signed the MLCommons CLA.
@patrick Fox
Patrick Fox seems not to be a GitHub user. You need a GitHub account after you become MLCommons member. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@gfursin
Copy link
Contributor

gfursin commented Nov 25, 2024

Hi @pnfox.

Thank you for your contribution to Collective Knowledge!

I can update the legacy CK framework even if it's archived.

However, the concept of CK/CM is to be backwards compatible.

In such case, I would add try/catch around imp and then import importlib in case of failure similar to what I did with setuptools here. Do you think it's possible to do it? I can then merge the PR and release a new version.

Another thing: can you please sign MLCommons CLA so that I could merge your PR?

By the way, out of curiosity, are you using CK? And if yes, how?

I am asking because we are developing a new version of CM/CK (called CMX) and plan to convert some legacy CK automations to CMX - such feedback can help us priorities our developments.

Thanks!

@pnfox pnfox closed this Nov 25, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants