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

[MNT] Configure mypy in pre-commit for type checking #423

Merged
merged 6 commits into from
Jan 25, 2025
Merged

Conversation

alyssadai
Copy link
Contributor

@alyssadai alyssadai commented Jan 24, 2025

Changes proposed in this pull request:

  • Configure mypy as part of pre-commit for type checking (see https://mypy.readthedocs.io/en/stable/command_line.html#the-mypy-command-line)
  • Expand return types for more complex data model utilities, for readability
  • Slightly refactor error handling to avoid implicit None return type
  • Fix type errors related to inference of None type objects (where such objects cannot actually be None due to preceding checks) by using direct indexing

Housekeeping:

  • Explicitly capture warning in test for failed pipeline catalog checking, to remove it from pytest warnings summary

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

Summary by Sourcery

Enhance the codebase by adding type annotations to several functions and integrate mypy into the pre-commit hooks to ensure type checking is performed.

Enhancements:

  • Add type annotations to various functions in the codebase to improve type safety and code clarity.

Build:

  • Integrate mypy into the pre-commit configuration to enforce type checking.

@alyssadai alyssadai added the skip-release PR doesn't appear in the changelog and preserves current version when merged label Jan 24, 2025
Copy link

sourcery-ai bot commented Jan 24, 2025

Reviewer's Guide by Sourcery

This pull request introduces mypy for static type checking by adding type annotations to function return types across several modules, refactoring error handling in the transform_age function, updating dictionary access methods for better error handling, and integrating mypy into the pre-commit hooks.

Class diagram showing updated type annotations

classDiagram
    class pheno_utils {
        +map_categories_to_columns(data_dict: dict) dict[str, list]
        +map_tools_to_columns(data_dict: dict) dict[str, list]
        +transform_age(value: str, heuristic: str) float
        +get_transformed_values(columns: list, row: pd.Series, data_dict: dict) list[float | str]
        +find_undefined_cat_col_values(data_dict: dict, pheno_df: pd.DataFrame) dict[str, list]
        +find_unused_missing_values(data_dict: dict, pheno_df: pd.DataFrame) dict[str, list]
    }
    class model_utils {
        +get_subject_instances(dataset: models.Dataset) dict[str, models.Subject]
        +get_imaging_session_instances(jsonld_subject: models.Subject) dict[str, models.ImagingSession]
    }
    class file_utils {
        +load_tabular(input_p: Path, input_type: str) pd.DataFrame
        +load_json(input_p: Path) Any
    }
    note for pheno_utils "Added type hints for dictionary and list return types"
    note for model_utils "Added specific type hints for subject and session dictionaries"
    note for file_utils "Updated return types for data loading functions"
Loading

File-Level Changes

Change Details Files
Add type annotations to function return types for better type checking and clarity.
  • Changed return type of map_categories_to_columns to dict[str, list].
  • Changed return type of map_tools_to_columns to dict[str, list].
  • Changed return type of get_transformed_values to list[float
str].
  • Changed return type of find_undefined_cat_col_values to dict[str, list].
  • Changed return type of find_unused_missing_values to dict[str, list].
  • Changed return type of get_subject_instances to dict[str, models.Subject].
  • Changed return type of get_imaging_session_instances to dict[str, models.ImagingSession].
  • Changed return type of load_tabular to pd.DataFrame.
  • Changed return type of load_json to Any.
  • Refactor error handling in transform_age function for better readability and efficiency.
    • Removed unnecessary variable is_recognized_heuristic.
    • Reorganized error handling to raise ValueError directly when heuristic is not recognized.
    bagel/utilities/pheno_utils.py
    Update dictionary access to use direct indexing instead of get method for better error handling.
    • Changed participants assignment to use direct indexing.
    • Changed existing_subject assignment to use direct indexing.
    • Changed existing_img_session assignment to use direct indexing.
    bagel/cli.py
    Add mypy to pre-commit hooks for static type checking.
    • Added mypy configuration to .pre-commit-config.yaml with specific arguments for type checking.
    .pre-commit-config.yaml

    Assessment against linked issues

    Issue Objective Addressed Explanation
    #403 Configure mypy pre-commit hook for type checking
    #403 Configure mypy to ignore untyped functions
    #403 Fix any type errors that are detected

    Possibly linked issues


    Tips and commands

    Interacting with Sourcery

    • Trigger a new review: Comment @sourcery-ai review on the pull request.
    • Continue discussions: Reply directly to Sourcery's review comments.
    • Generate a GitHub issue from a review comment: Ask Sourcery to create an
      issue from a review comment by replying to it. You can also reply to a
      review comment with @sourcery-ai issue to create an issue from it.
    • Generate a pull request title: Write @sourcery-ai anywhere in the pull
      request title to generate a title at any time. You can also comment
      @sourcery-ai title on the pull request to (re-)generate the title at any time.
    • Generate a pull request summary: Write @sourcery-ai summary anywhere in
      the pull request body to generate a PR summary at any time exactly where you
      want it. You can also comment @sourcery-ai summary on the pull request to
      (re-)generate the summary at any time.
    • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
      request to (re-)generate the reviewer's guide at any time.
    • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
      pull request to resolve all Sourcery comments. Useful if you've already
      addressed all the comments and don't want to see them anymore.
    • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
      request to dismiss all existing Sourcery reviews. Especially useful if you
      want to start fresh with a new review - don't forget to comment
      @sourcery-ai review to trigger a new review!
    • Generate a plan of action for an issue: Comment @sourcery-ai plan on
      an issue to generate a plan of action for it.

    Customizing Your Experience

    Access your dashboard to:

    • Enable or disable review features such as the Sourcery-generated pull request
      summary, the reviewer's guide, and others.
    • Change the review language.
    • Add, remove or edit custom review instructions.
    • Adjust other review settings.

    Getting Help

    Copy link

    codecov bot commented Jan 24, 2025

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 98.54%. Comparing base (9125b91) to head (0ca5554).
    Report is 3 commits behind head on main.

    Additional details and impacted files
    @@            Coverage Diff             @@
    ##             main     #423      +/-   ##
    ==========================================
    - Coverage   98.54%   98.54%   -0.01%     
    ==========================================
      Files          18       18              
      Lines        1101     1100       -1     
    ==========================================
    - Hits         1085     1084       -1     
      Misses         16       16              

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @alyssadai alyssadai added pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1) and removed skip-release PR doesn't appear in the changelog and preserves current version when merged labels Jan 24, 2025
    @alyssadai alyssadai marked this pull request as ready for review January 24, 2025 19:53
    @alyssadai alyssadai changed the title [MNT] Add mypy [MNT] Configure mypy in pre-commit for type checking Jan 24, 2025
    @rmanaem rmanaem self-requested a review January 24, 2025 23:32
    Copy link
    Contributor

    @rmanaem rmanaem left a comment

    Choose a reason for hiding this comment

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

    🧑‍🍳

    @alyssadai alyssadai merged commit 7b6e2ae into main Jan 25, 2025
    9 checks passed
    @alyssadai alyssadai deleted the add-mypy branch January 25, 2025 03:29
    Copy link
    Contributor

    🚀 PR was released in v0.4.1 🚀

    @neurobagel-bot neurobagel-bot bot added the released This issue/pull request has been released. label Jan 29, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1) released This issue/pull request has been released.
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Add mypy pre-commit hook for type checking
    2 participants