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.

Sign up for GitHub

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

Add mypy #51

Merged
merged 2 commits into from
Nov 30, 2024
Merged

Add mypy #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45305ce
Add mypy
jan-janssen Nov 30, 2024
78d6563
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .github/workflows/mypy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: MyPy

on:
push:
branches: [ main ]
pull_request:

jobs:
mypy:
runs-on: ubuntu-latest
steps:
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: "3.13"
architecture: x64
Comment on lines +15 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider using a stable Python version

Python 3.13 is currently in alpha and might lead to unstable CI results. Consider using Python 3.11 or 3.12 instead.

- python-version: "3.13"
+ python-version: "3.12"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
python-version: "3.13"
architecture: x64
python-version: "3.12"
architecture: x64

- name: Checkout
uses: actions/checkout@v4
- name: Install mypy
run: pip install mypy
Comment on lines +19 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add package installation step

MyPy needs to analyze the installed package. The current workflow only installs mypy but not the package itself.

      - name: Install mypy
-       run: pip install mypy
+       run: |
+         pip install mypy
+         pip install -e .
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Install mypy
run: pip install mypy
- name: Install mypy
run: |
pip install mypy
pip install -e .

- name: Test
run: mypy --ignore-missing-imports ${{ github.event.repository.name }}
Comment on lines +1 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add mypy configuration for consistent type checking

The repository lacks mypy configuration settings. Since the project uses pyproject.toml for other configurations, I recommend adding a [tool.mypy] section to maintain consistent type checking behavior. This is particularly important as the CI workflow runs mypy checks.

Key points to consider:

  • No mypy configuration found in pyproject.toml
  • No separate mypy.ini or setup.cfg files exist
  • Current CI runs with --ignore-missing-imports flag only
🔗 Analysis chain

Verify mypy configuration file existence

It would be beneficial to check if a mypy configuration file exists in the repository to ensure consistent type checking settings.

Let me check the content of pyproject.toml to see if it has any mypy configuration, even if not under the [tool.mypy] section.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for mypy config files
echo "Checking for mypy configuration files..."
fd -t f "mypy.ini|setup.cfg|pyproject.toml"

# If pyproject.toml exists, check for mypy section
if [ -f "pyproject.toml" ]; then
    echo "Checking pyproject.toml for mypy configuration..."
    cat pyproject.toml | grep -A 10 "\[tool.mypy\]"
fi

Length of output: 338

Script:

#!/bin/bash
# Display the full content of pyproject.toml
echo "Content of pyproject.toml:"
cat pyproject.toml

Length of output: 1414

🧰 Tools
🪛 yamllint (1.35.1)

[error] 22-22: no new line character at the end of file

(new-line-at-end-of-file)

10 changes: 5 additions & 5 deletions conda_subprocess/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

__version__ = _version.get_versions()["version"]
__all__ = [
Popen,
call,
check_call,
check_output,
run,
"Popen",
"call",
"check_call",
"check_output",
"run",
]
3 changes: 2 additions & 1 deletion conda_subprocess/process.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from conda.base.context import (
PREFIX_NAME_DISALLOWED_CHARS,
ROOT_ENV_NAME,
Context,
_first_writable_envs_dir,
context,
)
Expand Down Expand Up @@ -127,7 +128,7 @@ def _locate_prefix_by_name(name, envs_dirs=None):
raise EnvironmentNameNotFound(name)


def _validate_prefix_name(prefix_name: str, ctx: context, allow_base=True) -> str:
def _validate_prefix_name(prefix_name: str, ctx: Context, allow_base=True) -> str:
"""Run various validations to make sure prefix_name is valid"""
if PREFIX_NAME_DISALLOWED_CHARS.intersection(prefix_name):
raise CondaValueError(
Expand Down
Loading