Skip to content

Conversation

mgorsk1
Copy link
Contributor

@mgorsk1 mgorsk1 commented Aug 26, 2025

What kind of change does this PR introduce?

  • Bugfix
  • New Feature
  • Feature Improvment
  • Refactoring
  • Documentation
  • Other, please describe:

Description:

Fixes #89

Checklist:

  • I have read the CONTRIBUTING document.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@mgorsk1
Copy link
Contributor Author

mgorsk1 commented Aug 26, 2025

gentleman can you help me out prioritizing this fix? @lukasmasuch @raethlein @JanKalkan

@lukasmasuch lukasmasuch requested a review from Copilot August 28, 2025 22:03
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes issue #89 by replacing deprecated find_module calls with a new get_module helper function and updates the minimum Python version requirement from 3.6 to 3.9.

  • Introduces a new get_module helper function using find_spec instead of the deprecated find_module
  • Updates minimum Python version requirement from 3.6 to 3.9 in setup.py and README
  • Adds integration test coverage for the generate_docs function

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/lazydocs/generation.py Introduces get_module helper function and replaces deprecated find_module calls
setup.py Updates minimum Python version requirement from 3.6 to 3.9
README.md Updates documentation to reflect new Python 3.9+ requirement
tests/test_generation.py Adds integration test for generate_docs function

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +628 to +631
def get_module(loader, module_name: str) -> Optional[Any]:
spec = loader.find_spec(module_name)
if spec is None:
raise ImportError(f"Cannot find module {module_name}")
Copy link

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

The function may raise an AttributeError if spec.loader is None. Consider adding a null check for spec.loader before calling load_module.

Suggested change
def get_module(loader, module_name: str) -> Optional[Any]:
spec = loader.find_spec(module_name)
if spec is None:
raise ImportError(f"Cannot find module {module_name}")
raise ImportError(f"Cannot find module {module_name}")
if spec.loader is None:
raise ImportError(f"Cannot load module {module_name}: spec.loader is None")

Copilot uses AI. Check for mistakes.

spec = loader.find_spec(module_name)
if spec is None:
raise ImportError(f"Cannot find module {module_name}")
return spec.loader.load_module(spec.name)
Copy link

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

The load_module method is deprecated. Consider using importlib.util.module_from_spec and spec.loader.exec_module pattern that's already used in the main code paths.

Suggested change
return spec.loader.load_module(spec.name)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module

Copilot uses AI. Check for mistakes.

@lukasmasuch lukasmasuch merged commit fc1b6fe into ml-tooling:main Aug 28, 2025
1 of 3 checks passed
@mgorsk1 mgorsk1 deleted the fix/89 branch September 1, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

find_module deprecated in python 3.4, and removed from 3.12

2 participants