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

Fix Clippy Linting #806

Open
1 of 2 tasks
haixuanTao opened this issue Mar 5, 2025 · 9 comments
Open
1 of 2 tasks

Fix Clippy Linting #806

haixuanTao opened this issue Mar 5, 2025 · 9 comments
Labels
cli CLI good first issue Good for newcomers

Comments

@haixuanTao
Copy link
Collaborator

haixuanTao commented Mar 5, 2025

To test linting, just do:

cargo clippy --all --tests --examples 

Most of the times, clippy would propose changes that can be easily accepted. :)

An automated fix could look like:

TODO

  • apply clippy automatic fix
cargo clippy --all --tests --examples  --fix
  • Remove _bound() suffix into pyo3 0.23
@haixuanTao haixuanTao added the good first issue Good for newcomers label Mar 5, 2025
@github-actions github-actions bot added the cli CLI label Mar 5, 2025
@MunishMummadi
Copy link
Contributor

Hey @haixuanTao can i work on this issue.
My understanding is that you just want me to run the following commands and push the code :)

cargo clippy --all --tests --examples
cargo clippy --all --tests --examples  --fix

or do you want to me to address the issues as produced by the compiler

@haixuanTao
Copy link
Collaborator Author

Ahah, it is indeed, not very complicated 😊

But let's open a PR, and see if the CI/CD raise any unexpected warnings 😊

It's okay if the first PR do not fix all issues.

@Rustix69
Copy link

Rustix69 commented Mar 7, 2025

@haixuanTao Can I work on the issue ?

haixuanTao added a commit that referenced this issue Mar 7, 2025
As discussed in #806 this PR runs the clippy automatic fixes.
@haixuanTao
Copy link
Collaborator Author

In order to move forward with this issue.

We need to fix depreciation coming from the change of pyo3 0.23:

PyO3 0.23 completes the removal of the "GIL Refs" API in favour of the new "Bound" API introduced in PyO3 0.21.

With the removal of the old API, many "Bound" API functions which had been introduced with _bound suffixes no longer need the suffixes as these names have been freed up. For example, PyTuple::new_bound is now just PyTuple::new (the existing name remains but is deprecated).

To do this, could anyone, remove _bound suffix in all concerned rust code?

# The vim command would look like this
s/_bound//g

# Similarly replace:
s/to_object/into_pyobject/g

# Similarly replace:
s/to_py/into_pyobject/g

Thanks in advance.

To go even further we could modify: https://github.com/huggingface/pyo3-special-method-derive with the same changes. :)

@MunishMummadi
Copy link
Contributor

@haixuanTao Is there anyway to get assigned to an issue. It looks bad when you worked and someone else pushed a PR.

@haixuanTao
Copy link
Collaborator Author

The thing is that I don't really know the people who ask to get assign on the issue.

If you are afraid of working on something someone is already working on. I'm going to raise additional first issues so that it works for everyone.

@MunishMummadi
Copy link
Contributor

I completely understand what you are saying. I live in a different timezone so. No problem. I could have pushed a PR.

@haixuanTao
Copy link
Collaborator Author

I have created a new issues to add support for microsoft phi4 models: #814

It might be a more challenging one but I can assign it to you if you want?

@MunishMummadi
Copy link
Contributor

I looked it up and started working on it but it was assigned to someone else. No worries, I will try to stay on watch. I made an issue care to drop your views #822

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli CLI good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants