Add template Python module and tests - #2
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a template Python package with a calculator module implementing addition and division, along with corresponding unit tests. The review feedback suggests raising a standard ZeroDivisionError instead of a ValueError for division by zero to make the API more idiomatic, updating the tests to match this exception, and importing directly from python_dev instead of src.python_dev to properly leverage the src/ layout.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ValueError: If denominator is zero. | ||
| """ | ||
| if denominator == 0: | ||
| raise ValueError("denominator must not be zero") |
There was a problem hiding this comment.
In Python, division by zero is conventionally represented by raising a ZeroDivisionError rather than a ValueError. Using the standard built-in exception makes the API more idiomatic and allows callers to handle division errors using standard exception handling.
| ValueError: If denominator is zero. | |
| """ | |
| if denominator == 0: | |
| raise ValueError("denominator must not be zero") | |
| ZeroDivisionError: If denominator is zero. | |
| """ | |
| if denominator == 0: | |
| raise ZeroDivisionError("denominator must not be zero") |
| @@ -0,0 +1,16 @@ | |||
| import pytest | |||
|
|
|||
| from src.python_dev import add, divide | |||
There was a problem hiding this comment.
Importing from src.python_dev defeats the purpose of using a src/ layout. Tests should import directly from python_dev to ensure they are testing the package as it would be installed by users.
To support this with your current tooling, you can add the following configurations to pyproject.toml:
- Under
[tool.pytest.ini_options], addpythonpath = ["src"]sopytestcan find the package. - Under
[tool.mypy], addmypy_path = "src"somypycan resolve the imports.
| from src.python_dev import add, divide | |
| from python_dev import add, divide |
|
|
||
|
|
||
| def test_divide_rejects_zero_denominator() -> None: | ||
| with pytest.raises(ValueError, match="denominator must not be zero"): |
There was a problem hiding this comment.
Update the test assertion to expect ZeroDivisionError instead of ValueError to align with the idiomatic exception type for division by zero.
| with pytest.raises(ValueError, match="denominator must not be zero"): | |
| with pytest.raises(ZeroDivisionError, match="denominator must not be zero"): |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9ffeaf2fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,16 @@ | |||
| import pytest | |||
|
|
|||
| from src.python_dev import add, divide | |||
There was a problem hiding this comment.
Use one package name so mypy can resolve the module
In this repo, mypy . treats src/python_dev/__init__.py as python_dev, but this import also loads the same file as src.python_dev. With this line present, make check fails at the mypy step before type checking with Source file found twice under different module names: "python_dev" and "src.python_dev", so the required quality workflow is broken even though pytest passes. Use a single module naming scheme, such as making src an explicit package or importing python_dev consistently with the configured path.
Useful? React with 👍 / 👎.
Summary
src/python_devtemplate packageaddanddividefunctionsTest intent
This PR is intended to exercise the baseline checks added previously:
Notes
The previous tooling PR was already merged before these template files were added, so this is a follow-up PR that contains only the template Python code and tests.