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

Use named arguments in DaskFinder.find_spec #90

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pavoljuhas
Copy link

Use named arguments in DaskFinder.find_spec

Follow the base class argument names as in MetaPathFinder.find_spec per
https://docs.python.org/3.13/library/importlib.html#importlib.abc.MetaPathFinder.find_spec

This allows find_spec callers to use keyword arguments for path and target.

Follow the base class argument names as in `MetaPathFinder.find_spec` per
https://docs.python.org/3.13/library/importlib.html#importlib.abc.MetaPathFinder.find_spec

This allows find_spec callers to use keyword arguments for `path` and `target`.
@pavoljuhas pavoljuhas requested a review from a team as a code owner February 28, 2025 19:14
Copy link

copy-pr-bot bot commented Feb 28, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@pavoljuhas
Copy link
Author

pavoljuhas commented Feb 28, 2025

This fixes the following import error for Cirq due to mismatched arguments of DaskFinder.find_spec

# in fresh venv for Python 3.12
$ pip install cirq-core rapids-dask-dependency
$ python -c "import cirq"
...
  File "/tmp/t312/lib/python3.12/site-packages/cirq/_import.py", line 66, in find_spec
    spec = self.finder.find_spec(fullname, path=path, target=target)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: DaskFinder.find_spec() got an unexpected keyword argument 'path'

$ pip uninstall rapids-dask-dependency
$ python -c "import cirq"
# works as expected

xref: googlecolab/colabtools#5141
cc: @raydouglass

@pavoljuhas pavoljuhas changed the base branch from branch-25.04 to main February 28, 2025 19:27
@vyasr vyasr added bug Something isn't working non-breaking Introduces a non-breaking change labels Feb 28, 2025
@vyasr
Copy link
Contributor

vyasr commented Feb 28, 2025

Thanks for this fix @pavoljuhas! Do you know what version of RAPIDS is installed on the systems where this issue is being experienced? That changes the branch that we would merge this change into.

@pavoljuhas
Copy link
Author

pavoljuhas commented Feb 28, 2025

Do you know what version of RAPIDS is installed on the systems where this issue is being experienced?

The Colab environment where it shows has rapids_dask_dependency version 24.12.0.
The commit 2304660 which introduced the code is in all branches starting from branch-24.06, so ideally the fix would apply to all branches that are still maintained.

Thank you for quick response!

Copy link

@wence- wence- left a comment

Choose a reason for hiding this comment

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

Changes looks good to me. Also not sure exactly which branch this should go to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants