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

proper type annotation #409

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

proper type annotation #409

wants to merge 9 commits into from

Conversation

felipao-mx
Copy link
Member

@felipao-mx felipao-mx commented Sep 24, 2024

Improves type annotations for Resource class and subclasses, focusing on return types. Enhances type inference, allowing linters to accurately recognize return types from Resource methods and subclasses, removing the need for typing.cast()

Summary by CodeRabbit

Release Notes for Version 1.0.3

  • New Features

    • Enhanced type handling in various resource classes by simplifying return statements, improving code clarity and maintainability.
  • Bug Fixes

    • Streamlined the creation and update methods across multiple classes, ensuring consistent return types without unnecessary casting.
  • Documentation

    • Updated version number to reflect the new release (1.0.3).

Copy link

coderabbitai bot commented Sep 24, 2024

Walkthrough

The changes across various files in the cuenca/resources directory primarily involve the removal of the cast function from the return statements of multiple class methods. This adjustment affects methods related to resource creation, updating, and deactivation in several classes, including ApiKey, Arpc, CardActivation, User, and others.

The modifications streamline the return types by allowing methods to return their results directly from internal method calls without explicit type casting. This includes methods such as create, update, and deactivate. Additionally, some methods have updated their return type annotations to use covariant type variables for improved type safety.

In the cuenca/version.py file, the version number has been updated from '1.0.2' to '1.0.3', reflecting a new release.

Lastly, in the test file tests/resources/test_api_keys.py, the instantiation of the ApiKey object has been changed from using a class method to directly calling the constructor with unpacked dictionary arguments. Overall, these changes focus on simplifying code by removing unnecessary type casting while maintaining the existing functionality.

Possibly related PRs

  • update cuenca-validations #406: The changes in this PR involve the removal of type casting in various methods, similar to the main PR's focus on simplifying the get_balance function by eliminating explicit type casting.

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b602497) to head (c96049f).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #409   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           49        49           
  Lines         1111      1101   -10     
=========================================
- Hits          1111      1101   -10     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cuenca/__init__.py 100.00% <100.00%> (ø)
cuenca/resources/api_keys.py 100.00% <100.00%> (ø)
cuenca/resources/arpc.py 100.00% <100.00%> (ø)
cuenca/resources/base.py 100.00% <100.00%> (ø)
cuenca/resources/card_activations.py 100.00% <100.00%> (ø)
cuenca/resources/card_validations.py 100.00% <100.00%> (ø)
cuenca/resources/cards.py 100.00% <100.00%> (ø)
cuenca/resources/clabes.py 100.00% <100.00%> (ø)
cuenca/resources/curp_validations.py 100.00% <100.00%> (ø)
cuenca/resources/endpoints.py 100.00% <100.00%> (ø)
... and 20 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b602497...c96049f. Read the comment docs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 24

Outside diff range comments (9)
cuenca/resources/user_credentials.py (1)

Line range hint 1-43: Summary: Improved type handling and code simplicity

The changes in this file align well with the PR objective of proper type annotation. By removing the cast calls in both create and update methods, the code becomes more straightforward while maintaining type safety through the existing type annotations.

These modifications:

  1. Simplify the code by removing redundant type casting.
  2. Rely on the correct return types of the underlying _create and _update methods.
  3. Maintain the existing type annotations, which provide clear type information.

To further improve type safety and code clarity:

  1. Consider adding type annotations to the _create and _update methods if they don't already have them.
  2. If not already present, add a __init__ method with type annotations for all attributes of the UserCredential class.
  3. Consider using from __future__ import annotations at the top of the file to enable postponed evaluation of annotations, which can help with circular dependencies and improve readability.

These suggestions will enhance the overall type safety and readability of the UserCredential class.

cuenca/resources/kyc_validations.py (1)

Line range hint 26-41: LGTM: Improved type annotation in the create method.

The removal of the cast function simplifies the code while maintaining type safety through the method's return type annotation. This change aligns well with the PR objective of proper type annotation.

For even better clarity, consider adding a type annotation to the _create method if it doesn't already have one. This would make the type consistency more explicit throughout the class. For example:

@classmethod
def _create(cls, *args, **kwargs) -> 'KYCValidation':
    # implementation

This ensures that _create explicitly declares its return type, matching the annotation in the create method.

cuenca/resources/card_activations.py (1)

Line range hint 52-56: Consider removing cast from the card property for consistency

For consistency with the change made in the create method, consider removing the cast function from the card property. The type annotation on the property should be sufficient for type checking.

Here's a suggested change:

 @property
 def card(self) -> Optional[Card]:
     result = None
     if self.card_uri:
-        result = cast(Card, retrieve_uri(self.card_uri))
+        result = retrieve_uri(self.card_uri)
     return result

This change maintains the correct type annotation while simplifying the code.

Tools
Ruff

48-48: Missing type annotation for self in method

(ANN101)

cuenca/resources/user_logins.py (1)

Line range hint 1-48: Suggest consistent use of actual types for annotations.

To improve type hinting consistency and provide better IDE support, consider using actual types instead of string literals for type annotations throughout the file. This aligns with the PR objective of proper type annotation.

Apply the following changes:

  1. Update the create method's return type annotation:
-    ) -> 'UserLogin':
+    ) -> UserLogin:
  1. If there are any other occurrences of string literal type annotations in the file (e.g., in base classes or other methods not shown in this diff), update them similarly.

These changes will make the type annotations more consistent and easier for static type checkers and IDEs to process.

cuenca/resources/kyc_verifications.py (1)

Line range hint 45-51: Approved: Removal of redundant type casting and suggestion for consistency

The removal of the cast function from the return statement is a good improvement. This change:

  1. Simplifies the code.
  2. Relies on the existing return type hint -> 'KYCVerification' for type checking.
  3. Aligns with the PR objective of "proper type annotation".

The _update method (likely defined in the Updateable parent class) should already return an instance of KYCVerification, making the explicit cast unnecessary.

For consistency with the create method, consider moving the return statement to a single line:

-        req = KYCVerificationUpdateRequest(curp=curp)
-        return cls._update(id=kyc_id, **req.dict())
+        return cls._update(id=kyc_id, **KYCVerificationUpdateRequest(curp=curp).dict())

This change would make both methods more consistent in their implementation style.

cuenca/resources/api_keys.py (1)

Incomplete Removal of cast Function Detected

The verification script identified multiple instances of the cast function remaining in the codebase outside of cuenca/resources/api_keys.py. These residual usages indicate that the removal of cast is not yet comprehensive and may require additional updates to ensure consistency and prevent potential type-related issues.

Files with remaining cast usages:

  • cuenca/resources/base.py
  • cuenca/resources/users.py
  • cuenca/resources/wallet_transactions.py
  • cuenca/resources/transfers.py
  • cuenca/resources/card_activations.py
  • cuenca/resources/whatsapp_transfers.py
  • cuenca/resources/otps.py
  • cuenca/resources/card_validations.py
  • cuenca/resources/card_transactions.py
  • cuenca/resources/bill_payments.py
  • cuenca/resources/commissions.py
  • cuenca/resources/deposits.py
  • cuenca/resources/balance_entries.py
  • cuenca/__init__.py

Additionally, several methods continue to use string literal type annotations, which may need review to maintain consistency across the codebase.

Analysis chain

Line range hint 39-77: Verify the impact of removing cast on the broader codebase.

The consistent removal of the cast function across all methods suggests a broader change in the codebase, possibly in the base classes or the API response handling. While these changes simplify the code and remove unnecessary type casting, it's important to ensure that this doesn't introduce any unintended side effects.

To verify the impact of these changes, please run the following script:

This script will help identify:

  1. Any remaining usages of the cast function that might need similar updates.
  2. The return types of _create and _update methods in base classes to ensure they align with these changes.
  3. Other classes using string literal type annotations, to consider a consistent approach across the codebase.

Please review the results and consider updating other parts of the codebase if necessary to maintain consistency.


Adjust Verification Scripts to Correctly Target Python Files

The previous scripts encountered issues due to the unsupported --type python option in rg. To effectively verify the impact of removing cast, please run the following updated script:

This script has been adjusted to:

  1. Use Glob Patterns with rg: Replaced the --type python flag with -g "*.py" to correctly target Python files.
  2. Exclude Specific Files Appropriately: Ensured that the cuenca/resources/api_keys.py file is excluded from the cast function search.
  3. Refine ast-grep Patterns: Simplified the patterns to more accurately match class and method definitions without unnecessary complexity.

Please execute this script and provide the output for further analysis.


Correcting Verification Scripts for Accurate Results

The previous scripts encountered issues due to incorrect usage of rg options and malformed ast-grep patterns. Please run the following updated script to accurately verify the impact of removing cast:

This script includes the following corrections:

  1. Proper File Exclusion with rg:

    • Replaced the incorrect --ignore-file option with -g "!cuenca/resources/api_keys.py" to exclude the specified file from the search.
  2. Refined ast-grep Patterns:

    • Corrected the patterns to accurately match classes inheriting from Creatable and Updateable, and their respective _create and _update methods.
    • Ensured that the patterns are syntactically correct and do not include any extraneous text.
  3. Enhanced Readability and Separation:

    • Added distinct echo statements for each ast-grep test to improve clarity in the output.

Please execute this updated script and provide the output for further analysis.


Refining Verification Scripts to Resolve Pattern Errors

The previous script encountered syntax errors with ast-grep due to malformed patterns. To accurately verify the return types of _create and _update methods in Creatable and Updateable classes, please run the following updated script:

This updated script includes the following corrections:

  1. Corrected ast-grep Patterns:

    • Removed unintended text from the patterns to ensure valid syntax.
    • Accurately matches classes inheriting from Creatable and Updateable, along with their _create and _update methods.
  2. Separated Tests for Clarity:

    • Added distinct echo statements for each ast-grep test to improve output readability.

Please execute this updated script and provide the output for further analysis.


Fixing Shell Script Patterns for Accurate Verification

The previous shell scripts encountered issues with the ast-grep patterns, causing syntax errors. To accurately verify the return types of _create and _update methods in Creatable and Updateable classes, please run the following updated script:

Corrections Made:

  1. Corrected ast-grep Patterns:

    • Removed all unintended text from the patterns to ensure they are valid.
    • Ensured that the patterns accurately match classes inheriting from Creatable and Updateable, along with their respective _create and _update methods.
    • Used single quotes for the ast-grep patterns to prevent shell interpretation issues.
  2. Shell Script Syntax:

    • Ensured that each ast-grep pattern is enclosed within single quotes.
    • Removed any trailing periods or characters that may cause syntax errors.
  3. Maintained Test Separation:

    • Kept each test distinct with clear echo statements for better readability of the output.

Please execute this updated script and provide the output for further analysis.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other usages of 'cast' function in the codebase and verify the return types of base class methods.

# Test 1: Search for other usages of 'cast' function
echo "Searching for other usages of 'cast' function:"
rg --type python "cast\(" -g '!cuenca/resources/api_keys.py'

# Test 2: Check the return types of _create and _update methods in base classes
echo "Checking return types of _create and _update methods in base classes:"
ast-grep --lang python --pattern $'class $CLASS(Creatable):
  $$$
  def _create($$$):
    $$$
    return $_'

ast-grep --lang python --pattern $'class $CLASS(Updateable):
  $$$
  def _update($$$):
    $$$
    return $_'

# Test 3: Search for other classes using string literal type annotations
echo "Searching for other classes using string literal type annotations:"
rg --type python "def \w+\(.*\) -> '[A-Z]\w+'"

Length of output: 801


Script:

#!/bin/bash
# Description: Check for other usages of 'cast' function in the codebase and verify the return types of base class methods.

# Test 1: Search for other usages of 'cast' function
echo "Searching for other usages of 'cast' function:"
rg "cast\(" -g "*.py" --ignore-file cuenca/resources/api_keys.py

# Test 2: Check the return types of _create and _update methods in base classes
echo "Checking return types of _create and _update methods in base classes:"
ast-grep --lang python --pattern "class $_(Creatable):
    $$
    def _create($_):
        $$
        return $_"

ast-grep --lang python --pattern "class $_(Updateable):
    $$
    def _update($_):
        $$
        return $_"

# Test 3: Search for other classes using string literal type annotations
echo "Searching for other classes using string literal type annotations:"
rg "def \w+\(.*\) -> '[A-Z]\w+'" -g "*.py"

Length of output: 6586


Script:

#!/bin/bash
# Description: Check for other usages of 'cast' function in the codebase and verify the return types of base class methods.

# Test 1: Search for other usages of 'cast' function, excluding 'cuenca/resources/api_keys.py'
echo "Searching for other usages of 'cast' function:"
rg "cast\(" -g "*.py" -g "!cuenca/resources/api_keys.py"

# Test 2: Check the return types of _create and _update methods in base classes
echo "Checking return types of _create methods in Creatable classes:"
ast-grep --lang python --pattern "class $_(Creatable):
    $$
    def _create($_):
        $$
        return $_" .

echo "Checking return types of _update methods in Updateable classes:"
ast-grep --lang python --pattern "class $_(Updateable):
    $$
    def _update($_):
        $$
        return $_" .

# Test 3: Search for other classes using string literal type annotations
echo "Searching for other classes using string literal type annotations:"
rg "def \w+\(.*\) -> '[A-Z]\w+'" -g "*.py"

Length of output: 5027


Script:

#!/bin/bash
# Description: Check for other usages of 'cast' function in the codebase and verify the return types of base class methods.

# Test 1: Search for other usages of 'cast' function, excluding 'cuenca/resources/api_keys.py'
echo "Searching for other usages of 'cast' function:"
rg "cast\(" -g "*.py" -g "!cuenca/resources/api_keys.py"

# Test 2: Check the return types of _create methods in Creatable classes
echo "Checking return types of _create methods in Creatable classes:"
ast-grep --lang python --pattern "class $_(Creatable):
    $$
    def _create($_):
        $$
        return $_" .

# Test 3: Check the return types of _update methods in Updateable classes
echo "Checking return types of _update methods in Updateable classes:"
ast-grep --lang python --pattern "class $_(Updateable):
    $$
    def _update($_):
        $$
        return $_" .

# Test 4: Search for other classes using string literal type annotations
echo "Searching for other classes using string literal type annotations:"
rg "def \w+\(.*\) -> '[A-Z]\w+'" -g "*.py"

Length of output: 5027


Script:

#!/bin/bash
# Description: Check for other usages of 'cast' function in the codebase and verify the return types of base class methods.

# Test 1: Search for other usages of 'cast' function, excluding 'cuenca/resources/api_keys.py'
echo "Searching for other usages of 'cast' function:"
rg "cast\(" -g "*.py" -g "!cuenca/resources/api_keys.py"

# Test 2: Check the return types of _create methods in Creatable classes
echo "Checking return types of _create methods in Creatable classes:"
ast-grep --lang python --pattern 'class $_(Creatable):
    $$
    def _create($_):
        $$
        return $_' .

# Test 3: Check the return types of _update methods in Updateable classes
echo "Checking return types of _update methods in Updateable classes:"
ast-grep --lang python --pattern 'class $_(Updateable):
    $$
    def _update($_):
        $$
        return $_' .

# Test 4: Search for other classes using string literal type annotations
echo "Searching for other classes using string literal type annotations:"
rg "def \w+\(.*\) -> '[A-Z]\w+'" -g "*.py"

Length of output: 3276

Tools
Ruff

57-57: Unnecessary dict call (rewrite as a literal)

Rewrite as a literal

(C408)

cuenca/resources/transfers.py (2)

Line range hint 78-86: Consider consistent type handling in create_many method

While reviewing the changes in the create method, I noticed that the create_many method still uses cast('Transfer', transfer) on line 85. For consistency with the changes made to the create method, consider reviewing this usage as well.

If the decision is to remove cast for improved code clarity, you might want to apply the same change here. However, if cast is deemed necessary for type safety, it might be worth reconsidering its removal in the create method.

If you decide to remove the cast, the line could be changed to:

transfers['submitted'].append(transfer)

Please ensure that this change, if made, doesn't impact type safety or the overall objective of proper type annotation.

Tools
Ruff

75-75: Missing type annotation for cls in classmethod

(ANN102)


Line range hint 1-99: Overall type annotation usage

The file generally makes good use of type annotations, which aligns with the PR objective of "proper type annotation". However, the inconsistent use of cast (removed in create but retained in create_many) might need attention.

Consider the following points for improving type annotations throughout the file:

  1. Consistency: Ensure consistent use of type casting and annotations across similar methods.
  2. Return types: Verify that all method return types are explicitly annotated, especially for public methods.
  3. Variable annotations: Consider adding type annotations to local variables where their types might not be obvious.
  4. Use of Optional: The file correctly uses Optional for parameters that can be None. Ensure this is consistently applied.

To maintain and improve type safety, consider using a static type checker like mypy in your development process. This can help catch type-related issues early and ensure consistency in type annotations across the codebase.

Tools
Ruff

75-75: Missing type annotation for cls in classmethod

(ANN102)

cuenca/resources/users.py (1)

Line range hint 1-177: Summary: Improved type annotations and removed redundant casts

The changes in this file consistently remove unnecessary type casting, which aligns well with the PR objective of proper type annotation. This improves code clarity and reduces redundancy. Here's a summary of the improvements:

  1. Removed redundant cast() in the balance property.
  2. Removed redundant cast() in the create class method.
  3. Removed redundant cast() in the update class method.

These changes enhance the code quality by relying on the correct types returned by the methods rather than using explicit casts. To further improve the code, consider implementing the suggested type annotation improvements in the previous comments.

As a general recommendation, continue to review and improve type annotations throughout the codebase. This will enhance type safety, improve IDE support, and make the code more self-documenting. Consider using tools like mypy for static type checking to catch any potential type-related issues early in the development process.

Tools
Ruff

61-61: Missing type annotation for self in method

(ANN101)

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between b602497 and 1590a9e.

Files selected for processing (28)
  • cuenca/resources/api_keys.py (4 hunks)
  • cuenca/resources/arpc.py (2 hunks)
  • cuenca/resources/base.py (7 hunks)
  • cuenca/resources/card_activations.py (1 hunks)
  • cuenca/resources/cards.py (4 hunks)
  • cuenca/resources/clabes.py (2 hunks)
  • cuenca/resources/curp_validations.py (2 hunks)
  • cuenca/resources/endpoints.py (3 hunks)
  • cuenca/resources/file_batches.py (2 hunks)
  • cuenca/resources/files.py (2 hunks)
  • cuenca/resources/kyc_validations.py (2 hunks)
  • cuenca/resources/kyc_verifications.py (3 hunks)
  • cuenca/resources/limited_wallets.py (2 hunks)
  • cuenca/resources/login_tokens.py (2 hunks)
  • cuenca/resources/platforms.py (2 hunks)
  • cuenca/resources/questionnaires.py (2 hunks)
  • cuenca/resources/resources.py (2 hunks)
  • cuenca/resources/savings.py (3 hunks)
  • cuenca/resources/sessions.py (2 hunks)
  • cuenca/resources/transfers.py (1 hunks)
  • cuenca/resources/user_credentials.py (3 hunks)
  • cuenca/resources/user_lists_validation.py (2 hunks)
  • cuenca/resources/user_logins.py (2 hunks)
  • cuenca/resources/users.py (3 hunks)
  • cuenca/resources/verifications.py (3 hunks)
  • cuenca/resources/wallet_transactions.py (1 hunks)
  • cuenca/version.py (1 hunks)
  • tests/resources/test_api_keys.py (1 hunks)
Additional context used
Ruff
cuenca/resources/base.py

38-38: Trailing comma missing

Add trailing comma

(COM812)


43-43: Missing type annotation for self in method

(ANN101)


52-52: Missing type annotation for **data

(ANN003)


52-52: Trailing comma missing

Add trailing comma

(COM812)


64-64: Missing type annotation for **data

(ANN003)


64-64: Trailing comma missing

Add trailing comma

(COM812)


75-75: Missing type annotation for **data

(ANN003)


75-75: Trailing comma missing

Add trailing comma

(COM812)


81-81: Missing type annotation for self in method

(ANN101)


118-118: Missing type annotation for **data

(ANN003)


140-140: Missing type annotation for **query_params

(ANN003)


140-140: Trailing comma missing

Add trailing comma

(COM812)


154-154: Missing type annotation for **query_params

(ANN003)


154-154: Trailing comma missing

Add trailing comma

(COM812)


168-168: Missing type annotation for **query_params

(ANN003)


168-168: Trailing comma missing

Add trailing comma

(COM812)


176-176: Missing type annotation for **query_params

(ANN003)


176-176: Trailing comma missing

Add trailing comma

(COM812)

Additional comments not posted (39)
cuenca/version.py (2)

1-1: Version update looks good.

The version has been correctly updated from '1.0.2' to '1.0.3', following semantic versioning for a patch update.


1-3: Verify if type annotations were intended.

The PR objective mentions "proper type annotation", but no type annotations have been added to the variables in this file. Please confirm if type annotations were intended to be added to __version__, CLIENT_VERSION, and API_VERSION.

If type annotations are needed, consider updating the variables as follows:

__version__: str = '1.0.3'
CLIENT_VERSION: str = __version__
API_VERSION: str = '2020-03-19'
cuenca/resources/clabes.py (1)

12-14: Approve removal of cast, but suggest adding return type annotation

The removal of the cast function aligns with the PR objective of "proper type annotation". This change is good as it eliminates unnecessary type casting.

However, to fully achieve proper type annotation, please consider the following suggestions:

  1. Add a return type annotation to the create method. This will make the method's contract clearer and help with type checking.

  2. Verify the return type of the _create method in the parent class (likely Creatable) to ensure type consistency.

Here's a suggested improvement:

     @classmethod
-    def create(cls, session: Session = global_session):
+    def create(cls, session: Session = global_session) -> 'Clabe':
         return cls._create(session=session)

To verify the return type of the _create method, please run the following script:

This will help ensure that the return type of create matches the return type of _create.

Verification successful

Approve removal of cast and add return type annotation

The removal of the cast function aligns with the PR objective of "proper type annotation," eliminating unnecessary type casting.

To fully achieve proper type annotation, please consider the following:

  1. Add a return type annotation to the create method:

         @classmethod
    -    def create(cls, session: Session = global_session):
    +    def create(cls, session: Session = global_session) -> R_co:
             return cls._create(session=session)
  2. Ensure that R_co is correctly defined to represent the intended return type in the context of the Clabe class.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the return type of the _create method in the parent class

# Test: Search for the _create method definition in potential parent classes
rg --type python -A 5 'def _create\(' 

Length of output: 70


Script:

#!/bin/bash
# Description: Search for the _create method definition in parent classes using the correct file type

# Attempt 1: Using the correct --type identifier for Python
rg --type py -A 5 'def _create\('

# If the above fails, use a glob pattern to search within .py files
# rg 'def _create\(' --glob '*.py' -A 5

Length of output: 364

Tools
Ruff

13-13: Missing return type annotation for classmethod create

(ANN206)


13-13: Missing type annotation for cls in classmethod

(ANN102)

cuenca/resources/login_tokens.py (1)

20-20: Approved: Improved type handling by removing unnecessary cast

The change to directly return cls._create(session=session) without using a cast function is a good improvement. This aligns well with the PR objective of "proper type annotation". Here's why this change is beneficial:

  1. It simplifies the code by removing an unnecessary type cast.
  2. It relies on the type system to correctly infer the return type, which should be LoginToken based on the method's return type annotation.
  3. It potentially improves static type checking, as modern type checkers can often infer types correctly without explicit casts.
  4. It maintains consistency with the base Creatable class, assuming _create is defined there and returns the correct type.

This change demonstrates good practices in Python type hinting and should lead to more maintainable and type-safe code.

cuenca/resources/resources.py (3)

3-3: LGTM: Improved type annotations

The addition of Dict and List to the import statement from typing is appropriate and consistent with their usage in the file. This change, along with the existing type annotations, contributes to better type checking and code clarity.


Line range hint 1-24: Summary: Improved type annotations and code simplification

The changes in this file successfully achieve the PR objective of "proper type annotation". The removal of the cast function in retrieve_uri simplifies the code while relying on the type system's inference capabilities. The import statements have been updated to include necessary types, and existing type annotations are appropriate and helpful.

These changes contribute to better type checking, code clarity, and maintainability. Great job on improving the type annotations and simplifying the code!

Tools
Ruff

14-14: Avoid specifying long messages outside the exception class

(TRY003)


16-16: LGTM: Simplified return statement

The removal of the cast function simplifies the code while maintaining type safety. This change suggests that the type system can now correctly infer the return type without explicit casting, which aligns with the PR objective of "proper type annotation".

To ensure type consistency across the codebase, please run the following script:

This script will help verify that all resource classes implementing Retrievable have a retrieve method with a consistent return type.

cuenca/resources/file_batches.py (1)

25-25: Approved: Improved type annotation and code simplification

The removal of the cast function from the return statement is a positive change that aligns with the PR objective of "proper type annotation". This modification:

  1. Simplifies the code by removing redundant type casting.
  2. Maintains type safety through the existing return type annotation -> 'FileBatch'.
  3. Improves readability without sacrificing type information for static type checkers.

The change is correct and beneficial to the codebase.

cuenca/resources/wallet_transactions.py (1)

Line range hint 1-1: Verify if the cast import is still needed.

With the removal of the type cast in the create method, the cast import from typing might no longer be necessary. However, it's still used in the wallet property.

Let's verify if cast is used elsewhere in the file:

If the script confirms that cast is only used in the wallet property, consider keeping it. Otherwise, if it's completely unused, you can remove the import.

cuenca/resources/limited_wallets.py (1)

40-40: Approval: Improved type handling and code simplification

The removal of the explicit type cast cast('LimitedWallet', ...) is a positive change that aligns well with the PR objective of "proper type annotation". This modification offers several benefits:

  1. It simplifies the code by removing redundant type casting.
  2. It maintains type safety through the method's return type annotation -> 'LimitedWallet'.
  3. It demonstrates trust in the class hierarchy and the cls._create method's implementation.
  4. It adheres to Python's principle of "explicit is better than implicit" by relying on the natural type returned by cls._create.

This change improves code readability and maintainability while preserving type information for static type checkers.

cuenca/resources/user_credentials.py (2)

43-43: LGTM! Verify return type of cls._update()

The removal of the cast call simplifies the code and aligns with the PR objective of proper type annotation. The method signature already includes correct type annotations.

To ensure type consistency, please verify that cls._update() returns a UserCredential object. Run the following script to check the return type:

#!/bin/bash
# Description: Verify the return type of cls._update() in the UserCredential class

# Test: Search for the _update method implementation
ast-grep --lang python --pattern $'class UserCredential($$$):
  $$$
  def _update($$$):
    $$$
    return $$$'

If cls._update() doesn't return a UserCredential object, consider adding a type annotation to the _update method to ensure type consistency.


28-28: LGTM! Verify return type of cls._create()

The removal of the cast call simplifies the code and aligns with the PR objective of proper type annotation. The method signature already includes correct type annotations.

To ensure type consistency, please verify that cls._create() returns a UserCredential object. Run the following script to check the return type:

If cls._create() doesn't return a UserCredential object, consider adding a type annotation to the _create method to ensure type consistency.

cuenca/resources/kyc_validations.py (2)

Line range hint 1-24: LGTM: Imports and class definition are properly annotated.

The imports and class definition, including attribute type annotations, are correctly implemented. This aligns well with the PR objective of proper type annotation.


Line range hint 1-41: Summary: Successful implementation of proper type annotations

The changes in this file effectively improve type annotations, particularly in the create method. The removal of the cast function simplifies the code while maintaining type safety through return type annotations. This aligns perfectly with the PR objective of implementing proper type annotations.

The imports, class definition, and method signatures are all correctly annotated, contributing to a well-typed and clear codebase. Great job on improving the type safety and readability of the code!

cuenca/resources/card_activations.py (1)

Line range hint 1-56: Summary: Good progress on improving type annotations

The changes in this file align well with the PR objective of proper type annotation. The simplification in the create method improves code clarity while maintaining correct type information.

Two minor suggestions were made to further improve consistency:

  1. Remove the unused cast import.
  2. Remove the redundant cast in the card property.

Overall, these changes and suggestions contribute to better type annotation practices in the codebase.

Tools
Ruff

48-48: Missing type annotation for self in method

(ANN101)

cuenca/resources/user_lists_validation.py (1)

39-39: Excellent improvement in type annotation!

The removal of the cast function is a positive change. It aligns with the PR objective of "proper type annotation" and improves code clarity. Here's why this change is beneficial:

  1. The method already has a return type annotation -> 'UserListsValidation', making the cast function redundant.
  2. Removing cast simplifies the code without losing any type information.
  3. It relies on the type system to infer the correct return type, which is a good practice in modern Python development.

This change maintains type safety while reducing unnecessary code, which is an excellent improvement.

cuenca/resources/user_logins.py (1)

Line range hint 1-48: Summary: Approved changes with minor suggestions for improvement

The changes in this file align well with the PR objective of proper type annotation. The removal of redundant casting in the create method is a good improvement. The suggested minor changes to use actual types instead of string literals for type annotations will further enhance the consistency and effectiveness of type hinting in the file.

Overall, the modifications contribute to cleaner and more maintainable code with proper type annotations.

cuenca/resources/savings.py (2)

54-54: Approve changes

The removal of the cast function aligns with the PR objective of proper type annotation. This change is good as it removes unnecessary type casting, assuming that cls._update already returns the correct type.

The method already has an explicit return type annotation, which is excellent for type safety and code readability.


Line range hint 1-54: Overall approval with a minor suggestion

The changes in this file align well with the PR objective of proper type annotation. The removal of unnecessary cast functions in both create and update methods improves code clarity without compromising type safety.

Key points:

  1. Good use of type hints throughout the file.
  2. Proper input validation using SavingRequest and SavingUpdateRequest.
  3. Correct usage of class methods.

The only minor suggestion is to add an explicit return type annotation to the create method, as mentioned in a previous comment.

Great job on improving the type annotations and maintaining clean, well-structured code!

cuenca/resources/kyc_verifications.py (2)

41-42: Approved: Removal of redundant type casting

The removal of the cast function from the return statement is a good improvement. This change:

  1. Simplifies the code.
  2. Relies on the existing return type hint -> 'KYCVerification' for type checking.
  3. Aligns with the PR objective of "proper type annotation".

The _create method (likely defined in the Creatable parent class) should already return an instance of KYCVerification, making the explicit cast unnecessary.

Tools
Ruff

41-41: Missing type annotation for cls in classmethod

(ANN102)


Line range hint 1-51: Summary: Improved type annotations and code simplification

The changes in this file successfully achieve the PR objective of "proper type annotation" by:

  1. Removing redundant cast functions from both create and update methods.
  2. Relying on the existing return type hints (-> 'KYCVerification') for type checking.

These modifications result in cleaner, more maintainable code without sacrificing type safety. The changes are consistent across both methods and align well with Python's type hinting system.

Great job on improving the code quality!

Tools
Ruff

41-41: Missing type annotation for cls in classmethod

(ANN102)

cuenca/resources/sessions.py (1)

53-53: Approval: Improved type handling by removing redundant cast

The removal of cast('Session', ...) from the return statement is a positive change that aligns with the PR objective of "proper type annotation". This modification:

  1. Simplifies the code by removing a redundant operation.
  2. Relies on the type checker rather than runtime type casting, which is a good practice.
  3. Maintains type safety through the existing return type annotation -> 'Session'.

This change effectively improves the code without compromising type information or safety.

cuenca/resources/verifications.py (1)

Line range hint 1-57: Overall, good improvements to type annotations.

The changes in this file successfully remove unnecessary cast functions, simplifying the code and improving type annotations. This aligns well with the PR objective of "proper type annotation".

Key improvements:

  1. Simplified return statements in both create and verify methods.
  2. Reliance on the correct return types of _create and _update methods.

These changes make the code cleaner and more maintainable while preserving type safety. The suggestions to add type annotations to the parent class methods (_create and _update) would further enhance type safety and code clarity.

Tools
Ruff

45-45: Trailing comma missing

Add trailing comma

(COM812)

cuenca/resources/arpc.py (1)

55-55: Approval: Improved type annotation usage

The removal of cast('Arpc', ...) from the return statement is a positive change that aligns with the PR objective of "proper type annotation". This modification is beneficial for the following reasons:

  1. It simplifies the code without changing its functionality.
  2. It removes unnecessary use of cast, which is primarily meant for static type checking and doesn't perform runtime type conversion.
  3. The method's return type is already correctly annotated as 'Arpc' in the method signature, making the cast redundant.

This change demonstrates a better understanding and usage of Python's type hinting system.

cuenca/resources/files.py (2)

47-47: Approve the simplification, but verify _upload method's return type.

The removal of the cast function simplifies the code and aligns with the PR objective of proper type annotation. This change is good as it reduces unnecessary type casting.

However, to ensure type safety:

  1. Verify that cls._upload always returns an instance of the File class.
  2. If not already present, consider adding a return type annotation to the _upload method:
@classmethod
def _upload(cls, ...) -> 'File':
    # method implementation

This will help catch any potential type mismatches at compile-time rather than runtime.

To verify the _upload method's implementation and type annotations, run the following script:

#!/bin/bash
# Description: Check the _upload method implementation and type annotations

# Search for the _upload method definition
ast-grep --lang python --pattern $'class File($$$):
  $$$
  @classmethod
  def _upload(cls, $$$):
    $$$'

# Search for any type annotations on _upload
rg --type python '_upload.*\).*->'

Line range hint 1-70: Overall type annotations look good, with a minor suggestion.

The file demonstrates good use of type annotations throughout:

  • Proper imports from typing
  • Class variables and attributes are well-annotated
  • Methods have appropriate return type annotations

To further improve type safety:

  1. Verify that the download method (likely defined in a parent class) has proper type annotations:
def download(self, file_id: str) -> BytesIO:
    # method implementation

This ensures consistency with its usage in the file property.

To verify the download method's implementation and type annotations, run the following script:

Tools
Ruff

50-50: Missing type annotation for self in method

(ANN101)

cuenca/resources/platforms.py (1)

79-79: Approved: Improved type handling by removing redundant cast

The removal of the cast function from the return statement is a positive change that aligns with the PR objective of "proper type annotation". This modification:

  1. Simplifies the code by eliminating redundant type casting.
  2. Relies on Python's type inference and the method's return type annotation (-> 'Platform').
  3. Assumes correct implementation of the _create method in the parent Creatable class.

This change enhances code readability without compromising type safety, as long as the _create method is correctly implemented in the parent class.

cuenca/resources/transfers.py (1)

72-72: Clarify the removal of cast and its impact on type safety

The removal of cast(Transfer, ...) simplifies the code, but it might affect type checking and seems to contradict the PR objective of "proper type annotation". Please address the following points:

  1. Type safety: Can we guarantee that cls._create(**req.dict()) always returns a Transfer object? If not, removing cast might lead to less strict type checking.

  2. Return type consistency: The method is annotated to return 'Transfer'. Ensure that cls._create is consistent with this return type annotation.

  3. PR objective alignment: The PR aims for "proper type annotation". How does removing an explicit type cast align with this objective?

To help verify the return type of cls._create, please run the following script:

This will help us understand the implementation of _create and ensure type consistency.

Verification successful

Verified Removal of cast in create Method

The cls._create(**req.dict()) call in the create method is type-safe and consistently returns a Transfer instance by inheriting the _create method from the base class. Removing the explicit cast aligns with the PR objective of "proper type annotation" and simplifies the code without compromising functionality.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the implementation of _create method in the base classes

# Search for _create method in potential base classes
rg --type python -A 10 'def _create'

# Search for Transfer._create if it's defined in the class
ast-grep --lang python --pattern 'class Transfer($_) {
  $$$
  def _create($_) {
    $$$
  }
  $$$
}'

Length of output: 662


Script:

#!/bin/bash
# Description: Corrected scripts to check the implementation of _create method in base classes

# Search for _create method in potential base classes using the correct file type
rg --type py -A 10 'def _create'

# Search for Transfer._create within the Transfer class using a valid ast-grep pattern
ast-grep --lang python --pattern 'class Transfer {
  ...
  def _create($ARGS):
    ...
  ...
}'

Length of output: 1128

cuenca/resources/endpoints.py (2)

75-75: Approval: Improved type handling

The removal of the cast function from the return statement is a positive change. It simplifies the code and relies on Python's type system to infer the correct return type. This aligns well with the PR objective of "proper type annotation".

The method still correctly annotates its return type as 'Endpoint', which is sufficient for type checking purposes. This change maintains the intended functionality while improving code readability.


Line range hint 1-99: Summary: Improved type annotations and code clarity

The changes in this file successfully achieve the PR objective of "proper type annotation". By removing the cast function from both create and update methods, the code becomes cleaner and more idiomatic. These changes:

  1. Simplify the return statements in both methods.
  2. Rely on Python's type system to infer the correct types.
  3. Maintain the correct return type annotations.
  4. Improve code readability without affecting functionality.

These modifications align well with modern Python type hinting practices and should make the code easier to maintain and understand. The changes are subtle but impactful, demonstrating a good understanding of Python's type system.

cuenca/resources/cards.py (4)

Line range hint 1-120: Summary of changes and recommendations

The changes in this PR align well with the objective of "proper type annotation" by removing unnecessary cast functions. This improves code readability and relies more on Python's type inference capabilities. However, to further enhance type safety and code clarity, consider the following recommendations:

  1. Add explicit return type annotations to the create, update, and deactivate methods.
  2. In the deactivate method, consider adding a comment to explain the assumption made when using cls(**resp).

These changes will make the type hints more explicit and the code more self-documenting, which aligns with the goal of proper type annotation.


109-109: Verify return type annotation for update method

The removal of cast('Card', ...) improves code clarity. To ensure proper type annotation, we should explicitly annotate the return type of the method.

Please run the following script to check the method signature:

#!/bin/bash
# Description: Check the update method signature for proper return type annotation

ast-grep --lang python --pattern 'def update($$$) -> $_: $$$'

If the script doesn't return any results, consider updating the method signature to include an explicit return type annotation:

@classmethod
def update(
    cls,
    card_id: str,
    status: Optional[CardStatus] = None,
    pin_block: Optional[str] = None,
    is_dynamic_cvv: bool = False,
    *,
    session: Session = global_session,
) -> 'Card':
    # ... method body ...

120-120: Verify return type annotation and review cls(**resp) usage

  1. To ensure proper type annotation, we should explicitly annotate the return type of the method.

Please run the following script to check the method signature:

#!/bin/bash
# Description: Check the deactivate method signature for proper return type annotation

ast-grep --lang python --pattern 'def deactivate($$$) -> $_: $$$'

If the script doesn't return any results, consider updating the method signature to include an explicit return type annotation:

@classmethod
def deactivate(
    cls, card_id: str, *, session: Session = global_session
) -> 'Card':
    # ... method body ...
  1. The use of cls(**resp) assumes that the resp dictionary keys match the class attributes exactly. While this is likely the case, it's worth noting that this approach might be less robust than the previous _from_dict method if the API response structure changes.

Consider adding a comment to explain this assumption:

# Assume that the API response keys match the Card class attributes
return cls(**resp)

84-84: Verify return type annotation for create method

The removal of cast('Card', ...) is a good step towards cleaner code. However, to ensure proper type annotation, we should explicitly annotate the return type of the method.

Please run the following script to check the method signature:

If the script doesn't return any results, consider updating the method signature to include an explicit return type annotation:

@classmethod
def create(
    cls,
    issuer: CardIssuer,
    funding_type: CardFundingType,
    user_id: str = 'me',
    card_holder_user_id: Optional[str] = None,
    is_dynamic_cvv: bool = False,
    *,
    session: Session = global_session,
) -> 'Card':
    # ... method body ...
Verification successful

Return type annotation for create method is correctly specified

The create method in cuenca/resources/cards.py is properly annotated with an explicit return type 'Card'.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the create method signature for proper return type annotation

ast-grep --lang python --pattern 'def create($$$) -> $_: $$$'

Length of output: 2134


Script:

#!/bin/bash
# Description: Verify if the create method in cuenca/resources/cards.py has an explicit return type annotation

ast-grep --lang python --pattern 'def create($$$) -> $_: $$$' cuenca/resources/cards.py

Length of output: 1777

cuenca/resources/curp_validations.py (2)

Line range hint 1-101: Overall type annotations in the file look good

The file demonstrates consistent use of type annotations throughout the CurpValidation class. All attributes and method parameters have appropriate type hints, which contributes to code clarity and helps prevent type-related errors. The Config inner class provides useful field descriptions and an example, further enhancing the documentation of the class.


101-101: Approved: Removal of explicit cast improves type annotations

The removal of the cast function from the return statement is a good change. It aligns with the PR objective of "proper type annotation". The return type hint 'CurpValidation' in the method signature should be sufficient for type checking without explicit casting.

Let's verify the return type of the _create method to ensure it's compatible with CurpValidation:

This script will help us confirm that the _create method's return type is compatible with CurpValidation, ensuring that the removal of the explicit cast doesn't introduce any type-related issues.

Verification successful

Verified: _create method's return type is compatible with CurpValidation

The analysis confirms that the _create method in the Creatable class returns an instance compatible with CurpValidation. The removal of the explicit cast function aligns with proper type annotations and does not introduce any type-related issues.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the return type of the _create method in the Creatable class

# Search for the Creatable class definition
echo "Searching for Creatable class definition:"
rg --type python -A 10 "class Creatable"

# Search for the _create method definition
echo "\nSearching for _create method definition:"
rg --type python -A 5 "def _create"

# Search for any type hints related to _create method
echo "\nSearching for type hints related to _create method:"
rg --type python "_create.*->|_create.*:.*="

Length of output: 525


Script:

#!/bin/bash
# Description: Check the return type of the _create method in the Creatable class

# Search for the Creatable class definition
echo "Searching for Creatable class definition:"
rg --type py -A 10 "class Creatable"

# Search for the _create method definition
echo "\nSearching for _create method definition:"
rg --type py -A 5 "def _create"

# Search for any type hints related to _create method
echo "\nSearching for type hints related to _create method:"
rg --type py "_create.*->|_create.*:.*="

Length of output: 1456

cuenca/resources/users.py (1)

138-138: Approved: Removal of unnecessary type casting

The removal of cast() in the create method is appropriate and aligns with the PR objective of proper type annotation. The _create method should already return a User object, making the cast redundant.

cuenca/resources/base.py (2)

20-20: Introduction of covariant 'R_co' enhances type safety

The addition of R_co = TypeVar('R_co', bound='Resource', covariant=True) improves type safety and ensures that type annotations are correctly inferred in methods returning instances of resources.


28-29: Setting 'extra = Extra.ignore' in 'Config' is appropriate

By configuring Pydantic's Config with extra = Extra.ignore, any unexpected fields in the data will be ignored. This prevents potential errors when parsing API responses that include additional fields not defined in the model.

cuenca/resources/questionnaires.py Show resolved Hide resolved
cuenca/resources/wallet_transactions.py Show resolved Hide resolved
cuenca/resources/card_activations.py Show resolved Hide resolved
cuenca/resources/user_logins.py Show resolved Hide resolved
cuenca/resources/savings.py Show resolved Hide resolved
cuenca/resources/base.py Outdated Show resolved Hide resolved
cuenca/resources/base.py Outdated Show resolved Hide resolved
cuenca/resources/base.py Outdated Show resolved Hide resolved
cuenca/resources/base.py Outdated Show resolved Hide resolved
cuenca/resources/base.py Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 14

Outside diff range comments (2)
cuenca/resources/transfers.py (1)

Line range hint 1-95: Overall improvement in type annotations with suggestion for broader review

The changes in this file successfully remove unnecessary type casting in the create and create_many methods, aligning well with the PR objectives. These modifications will improve type inference for linters and make the code more maintainable.

To further enhance the type safety across the codebase:

  1. Review parent classes (Transaction, Creatable) and ensure their methods have appropriate return type annotations.
  2. Check if similar type casting removals can be applied to other resource classes that might have a similar structure.
  3. Consider adding type annotations to class variables and properties if they're missing.

These additional steps would ensure consistency across the entire resource module and maximize the benefits of the improved type annotations.

Tools
Ruff

75-75: Missing type annotation for cls in classmethod

(ANN102)

cuenca/resources/endpoints.py (1)

Line range hint 78-100: LGTM: Improved type handling and documentation

The changes in the update method are positive:

  1. Removing the cast function from the return statement simplifies the code and improves readability.
  2. Adding the events parameter to the method signature and docstring enhances the documentation.

These changes align well with the PR objective of "proper type annotation" and maintain the intended functionality.

Minor suggestion: Add a trailing comma after the last item in the EndpointUpdateRequest constructor for consistency with Python style guidelines:

 req = EndpointUpdateRequest(
-    url=url, is_enable=is_enable, events=events
+    url=url, is_enable=is_enable, events=events,
 )
Tools
Ruff

98-98: Trailing comma missing

Add trailing comma

(COM812)

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 1590a9e and c96049f.

Files selected for processing (9)
  • cuenca/init.py (1 hunks)
  • cuenca/resources/base.py (7 hunks)
  • cuenca/resources/card_validations.py (1 hunks)
  • cuenca/resources/endpoints.py (3 hunks)
  • cuenca/resources/otps.py (2 hunks)
  • cuenca/resources/savings.py (3 hunks)
  • cuenca/resources/transfers.py (2 hunks)
  • cuenca/resources/users.py (4 hunks)
  • cuenca/resources/wallet_transactions.py (1 hunks)
Additional context used
Ruff
cuenca/resources/base.py

46-46: Missing type annotation for self in method

(ANN101)


58-58: Dynamically typed expressions (typing.Any) are disallowed in **data

(ANN401)


74-74: Dynamically typed expressions (typing.Any) are disallowed in **data

(ANN401)


89-89: Dynamically typed expressions (typing.Any) are disallowed in **data

(ANN401)


95-95: Missing type annotation for self in method

(ANN101)


132-132: Dynamically typed expressions (typing.Any) are disallowed in **data

(ANN401)


157-157: Dynamically typed expressions (typing.Any) are disallowed in **query_params

(ANN401)


174-174: Dynamically typed expressions (typing.Any) are disallowed in **query_params

(ANN401)


191-191: Dynamically typed expressions (typing.Any) are disallowed in **query_params

(ANN401)


202-202: Dynamically typed expressions (typing.Any) are disallowed in **query_params

(ANN401)

cuenca/resources/endpoints.py

98-98: Trailing comma missing

Add trailing comma

(COM812)

Additional comments not posted (11)
cuenca/resources/otps.py (3)

1-1: LGTM: Removal of unused import

The removal of the unused cast import from the typing module is a good cleanup. This change aligns with the PR objective of eliminating the necessity for using typing.cast() and helps in keeping the imports clean and relevant.


Line range hint 1-24: Summary: Improved type handling in Otp class

The changes in this file successfully implement the PR objective of enhancing type annotations and eliminating unnecessary typing.cast() usage. The create method now returns the result of cls._create(session=session) directly, which simplifies the code while maintaining type safety through the return type annotation. These modifications should improve code readability and maintainability without compromising type inference capabilities.


Line range hint 19-24: LGTM: Simplified return statement in create method

The removal of the cast function in the return statement aligns with the PR objective of eliminating the necessity for using typing.cast(). This change simplifies the code and improves readability while maintaining the return type annotation -> 'Otp' for static type checking.

To ensure this change doesn't negatively impact type inference elsewhere in the codebase, please run the following verification script:

This script will help identify any potential issues with type inference that might have been introduced by this change.

cuenca/resources/wallet_transactions.py (1)

32-32: LGTM! Great improvements to type annotations.

The changes in the create method are excellent:

  1. The addition of the explicit return type annotation -> 'WalletTransaction' improves code clarity and type safety.
  2. The removal of the type cast in the return statement aligns with the PR objective of eliminating unnecessary typing.cast() usage.

These changes maintain the existing functionality while enhancing type inference and code readability. The suggestion from the past review comment has been successfully addressed.

Also applies to: 38-38

cuenca/resources/savings.py (1)

30-37: Excellent implementation of proper type annotations!

The changes in the create method align perfectly with the PR objectives:

  1. The removal of the cast function simplifies the code and improves readability.
  2. The addition of the -> 'Saving' return type annotation enhances type safety.

These modifications will allow for better type inference and eliminate the need for explicit casting. Great job implementing the suggestion from the previous review!

cuenca/__init__.py (1)

97-97: Excellent improvement in type handling!

The removal of the explicit cast() function aligns well with the PR objectives. This change:

  1. Simplifies the code, making it more readable and maintainable.
  2. Indicates improved type inference, likely due to enhanced type annotations in the BalanceEntry class.
  3. Maintains the existing functionality while potentially offering a slight performance improvement by eliminating the casting overhead.

Great job on streamlining the code while preserving its behavior!

cuenca/resources/card_validations.py (1)

54-54: Approve the removal of unnecessary type casting

The removal of the cast function from the return statement aligns with the PR objective of improving type annotations and simplifying the code. This change enhances readability and maintainability.

To ensure type safety, let's verify the return type annotation of the _create method:

This will help confirm that the _create method is properly annotated to return a CardValidation object, ensuring type safety without the need for explicit casting.

cuenca/resources/transfers.py (1)

72-72: Approved: Improved type inference by removing explicit casting

The removal of cast(Transfer, ...) aligns well with the PR objectives. This change will allow for better type inference by linters and eliminates the need for explicit type casting. The functionality remains unchanged, which is excellent for maintaining backwards compatibility.

cuenca/resources/endpoints.py (1)

Line range hint 64-75: LGTM: Improved type handling

The removal of the cast function from the return statement is a positive change. It simplifies the code and relies on Python's type system to infer the correct return type. This aligns well with the PR objective of "proper type annotation".

The method still correctly annotates its return type as 'Endpoint', which is sufficient for type checking purposes. This change maintains the intended functionality while improving code readability.

cuenca/resources/users.py (1)

Line range hint 1-182: Summary: Improvements in type annotations with suggestions for further enhancements

The changes in this file successfully remove unnecessary type casting, aligning with the PR objective of improving type annotations. These modifications simplify the code and make it more maintainable.

To further enhance the type safety and readability of the code, consider implementing the suggested return type annotations for the balance property, create, and update methods.

Overall, these changes represent a positive step towards better type handling in the User class.

Tools
Ruff

61-61: Missing type annotation for self in method

(ANN101)

cuenca/resources/base.py (1)

38-42: Improved type safety in retrieve method

The updated method signature of retrieve now uses cls: Type[R_co] and returns R_co, leveraging the covariant type variable. This enhances type safety and clarity.

cuenca/resources/savings.py Show resolved Hide resolved
cuenca/resources/transfers.py Show resolved Hide resolved
Comment on lines +62 to 63
be = BalanceEntry.first(user_id=self.id)
return be.rolling_balance if be else 0
Copy link

Choose a reason for hiding this comment

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

Approved: Removal of unnecessary type casting with suggestion for further improvement

The removal of cast() is appropriate and aligns with the PR objective of proper type annotation. This change simplifies the code without affecting its functionality.

Consider updating the return type annotation of the balance property to be more specific:

@property
def balance(self) -> int:
    be = BalanceEntry.first(user_id=self.id)
    return be.rolling_balance if be else 0

This change would make the type annotation more precise and consistent with the actual return value.

cuenca/resources/users.py Show resolved Hide resolved
'User',
cls._update(id=user_id, **request.dict(), session=session),
)
return cls._update(id=user_id, **request.dict(), session=session)
Copy link

Choose a reason for hiding this comment

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

Approved: Removal of unnecessary type casting with suggestion for further improvement

The removal of cast() in the update method is appropriate and aligns with the PR objective of proper type annotation. This change simplifies the code without affecting its functionality.

Consider adding an explicit return type annotation to the update method:

@classmethod
def update(
    cls,
    user_id: str,
    # ... other parameters ...
) -> 'User':
    # ... method body ...
    return cls._update(id=user_id, **request.dict(), session=session)

This change would make the return type explicit and consistent with the actual return value.

cuenca/resources/base.py Show resolved Hide resolved
cuenca/resources/base.py Show resolved Hide resolved
cuenca/resources/base.py Show resolved Hide resolved
cuenca/resources/base.py Show resolved Hide resolved
cuenca/resources/base.py Show resolved Hide resolved
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.

1 participant