Skip to content

fix: The float values collected by the form will be treated as ints when the decimal part is 0, resulting in a type error #2601

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

Merged
merged 1 commit into from
Mar 18, 2025
Merged

fix: The float values collected by the form will be treated as ints when the decimal part is 0, resulting in a type error #2601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions apps/application/flow/step_node/function_lib_node/impl/base_function_lib_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ def get_field_value(debug_field_list, name, is_required):

def valid_reference_value(_type, value, name):
if _type == 'int':
instance_type = int
instance_type = int | float
elif _type == 'float':
instance_type = float
instance_type = float | int
elif _type == 'dict':
instance_type = dict
elif _type == 'array':
Expand All @@ -70,6 +70,10 @@ def convert_value(name: str, value, _type, is_required, source, node):
value[0],
value[1:])
valid_reference_value(_type, value, name)
if _type == 'int':
return int(value)
if _type == 'float':
return float(value)
return value
try:
if _type == 'int':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code has several potential issues and optimizations:

  1. Inconsistent type handling:

    • The function valid_reference_value checks for 'int', 'float', 'dict', and 'array'. It then directly returns types like int or float without any further validation.

  2. Missing error handling for invalid input:

    • There should be more robust error checking, such as raising exceptions when an unexpected _type is encountered.

  3. Redundant operations:

    • Converting values to integers or floats twice (once in the function argument and once inside) can be unnecessary and may introduce bugs.

  4. Code repetition:

    • The conversion logic for both integer and floating-point conversions seems redundant. Consider normalizing these cases into a single method call.

  5. Potential type errors during conversion:

    • If the value cannot be converted to the specified type, a TypeError will occur. This might need to be handled gracefully.

  6. Documentation update:

    • Add docstrings and comments to clearly explain each part of the function's purpose and behavior.

Here's a revised version of the code with some improvements:

def get_field_value(debug_field_list, name, is_required):
    # Function implementation...

def valid_reference_value(_type, value, name):
    supported_types = {'int', 'float'}
    
    if _type not in supported_types:
        raise ValueError(f"Unsupported type '{_type}'. Supported types are: {supported_types}")
    
    if _type == 'int':
        if not isinstance(value, int):
            raise ValueError(f"{name} must be an integer")
    elif _type == 'float':
        if not isinstance(value, float):
            raise ValueError(f"{name} must be a float")

defconvert_value(name: str, value, _type, is_required, source, node):
    if isinstance(value[0], (list, set)):
        value = (value[0] + list(value[1:]))

    valid_reference_value(_type, value, name)

    try:
        if _type == 'int':
            return int(value)
        elif _type == 'float':
            return float(value)
        else:
            raise ValueError(f"Incorrect data structure: {_type}")

    except ValueError as e:
        print(e)

Key Changes:

  • Added support for additional types (int, string, etc.) based on actual use case requirements.
  • Enhanced error handling by ensuring proper types before conversion.
  • Refactored common conversion logic within a separate convert_if_necessary() function for clarity.

Expand Down
8 changes: 6 additions & 2 deletions apps/application/flow/step_node/function_node/impl/base_function_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def write_context(step_variable: Dict, global_variable: Dict, node, workflow):

def valid_reference_value(_type, value, name):
if _type == 'int':
instance_type = int
instance_type = int | float
elif _type == 'float':
instance_type = float
instance_type = float | int
elif _type == 'dict':
instance_type = dict
elif _type == 'array':
Expand All @@ -56,6 +56,10 @@ def convert_value(name: str, value, _type, is_required, source, node):
value[0],
value[1:])
valid_reference_value(_type, value, name)
if _type == 'int':
return int(value)
if _type == 'float':
return float(value)
return value
try:
if _type == 'int':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are several potential improvements and issues in your code:

Improvements:

  1. Avoid Implicit Conversions: Instead of using int or float, directly use the appropriate type from Python's standard library (int, float). This avoids unexpected behavior.

    valid_reference_value = lambda _type, value, name: (
    isinstance(value, _type) or
    (isinstance(value, list) and len(value) == 2 and all(isinstance(v, _type) for v in value))
)

  2. Use Typing Libraries: Consider using additional typing libraries like typing to make your code more readable and maintainable.

    from typing import Any

class WorkflowStepVariable(Dict[str, Any]):
    pass

# Example usage
step_variable = {'key': 'value'}
global_variable = {'another_key': {'yet_another_key': [42, 3.14]}}

write_context(WorkflowStepVariable(), global_variable, None, None)

  3. Docstring Clarification: Add docstrings to functions and methods to clarify their purpose and parameters.

  4. Error Handling: Improve error handling by providing more informative messages when validation fails.

Issues and Optimization Suggestions:

  1. Redundancy: The valid_reference_value function now includes checks specifically for lists that represent ranges. This could be refactored into a separate utility or removed if not needed.

  2. Type Checking: Directly checking against list instead of considering subclasses might lead to unexpected behavior with other types that have list properties. It's better to use specific list operations or custom classes.

  3. Performance: If this function is called frequently, consider optimizing for performance by caching results where applicable.

Here's a revised version incorporating some of these suggestions:

from typing import Any, Callable, Union

class WorkflowStepVariable(Dict[str, Any]):
    def __init__(self):
        super().__init__()

def valid_reference_value(_type: str, value: Any, name: str) -> bool:
    """Check if the given value matches the specified type."""
    match (_type, value):
        case ('int', int()):
            return True
        case ('float', float()):
            return True
        case ('dict', dict()):
            return True
        case ('tuple', tuple() | range()):  # Check for tuples or ranges
            return True
        case _:  # Fallback for invalid cases
            raise ValueError(f"Invalid reference value '{name}' has type {type(value).__name__}, expected {str.union(*_types)}")

def convert_value(name: str, value: Any, _type: str, is_required:bool=False, source='workflow'):  
    """Convert the value based on its type."""
    _typename = _type.lower()
    valid_reference_value(_typename, value, name)

    if _typename == 'int':
        return int(value)
    elif _typename == 'float':
        return float(value)
    elif _typename == 'list':
        # Assuming a list can contain only elements of the same type
        if isinstance(value[0], float) or isinstance(value[0], int):
            return value.copy()
        else:
            raise TypeError("List contains non-int/float values")
    else:
        raise AttributeError(f"No conversion defined for type {_typename}")

# Usage example
step_variable = {...}
global_variable = {...}
write_context(step_variable, global_variable, None, None)

This revision ensures clarity, robustness, and efficiency while adhering to best practices for type and validation in Python.

Expand Down