Use copilot self-review suggestion for pkg_resources replacement#66
Use copilot self-review suggestion for pkg_resources replacement#66
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the importlib.resources.files approach with importlib.util.find_spec in the add_py_module function to achieve consistency with the existing add_numpy function implementation.
Changes:
- Modified
get_module_include_pathfunction withinadd_py_moduleto usefind_specandsubmodule_search_locationsinstead ofimportlib.resources.files
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from importlib.util import find_spec | ||
| spec = find_spec(name) | ||
| if spec is None: | ||
| raise RuntimeError(f"Could not find module '{name}'") | ||
| libdir = spec.submodule_search_locations | ||
| assert libdir is not None | ||
| from os.path import join | ||
| return join(libdir[0], "include") |
There was a problem hiding this comment.
The change from importlib.resources.files to find_spec with submodule_search_locations makes this function consistent with add_numpy above, but moves away from the more modern and robust approach. The importlib.resources.files API is the recommended way to access package resources and works with any import mechanism including zip files, while the new approach assumes filesystem-based packages. For this use case (finding C/C++ include directories), filesystem-based packages are expected, so the practical impact is minimal. However, if consistency is the goal, consider updating add_numpy to also use importlib.resources.files for better maintainability.
1 participant
No description provided.