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

is_compatible_with edge cases #1190

Open
jules-ch opened this issue Oct 15, 2020 · 2 comments · May be fixed by #1191
Open

is_compatible_with edge cases #1190

jules-ch opened this issue Oct 15, 2020 · 2 comments · May be fixed by #1191

Comments

@jules-ch
Copy link
Collaborator

Following on is_compatible_with, I found several edge cases.

Working with dimensionless with first argument :

>>> ureg.is_compatible_with(np.array([1, 2, 3, 4]), ureg.deg)
False

Expected True
>>> ureg.is_compatible_with(ureg.deg, 10.0)
True
>>> ureg.is_compatible_with(10.0, ureg.deg)
False

Expected True

With string as arguments:

>>>ureg.is_compatible_with("80 in", "35000 ft")
ValueError: Unit expression cannot have a scaling factor.
>>>ureg.is_compatible_with("in", "35000 ft")
ValueError: Unit expression cannot have a scaling factor.

For this error, we need to use parse_expression on Unit.is_compatible_with & Quantity.is_compatible_with instead of parse_units.

I've rewrited the function like so:

    def is_compatible_with(
        self, obj1, obj2, *contexts, **ctx_kwargs
    ):
        """ check if the other object is compatible

        Parameters
        ----------
        obj1, obj2
            The objects to check against each other. Treated as
            dimensionless if not a Quantity, Unit or str.
        *contexts : str or pint.Context
            Contexts to use in the transformation.
        **ctx_kwargs :
            Values for the Context/s

        Returns
        -------
        bool
        """
        if isinstance(obj1, (dict, bool, type(None))) or isinstance(obj2, (dict, bool, type(None))):
             raise TypeError("Type can't be casted to Quantity")

        if isinstance(obj1, (self.Quantity, self.Unit)):
            return obj1.is_compatible_with(obj2, *contexts, **ctx_kwargs)

        if isinstance(obj1, str):
            return self.parse_expression(obj1).is_compatible_with(
                obj2, *contexts, **ctx_kwargs
            )

        if isinstance(obj2, (self.Quantity, self.Unit, str)):
            return self.is_compatible_with(obj2, obj1, *contexts, **ctx_kwargs)
        else:
            return self.Quantity(obj1).is_compatible_with(obj2, *contexts, **ctx_kwargs)

Not sure if we should throw an error on types that can't be casted to Quantity or return simply False.

@keewis if you want to chime in

jules-ch added a commit to jules-ch/pint that referenced this issue Oct 15, 2020
Make is_compatible_with function with dimensionless types & string parsing errors.
is_compatible_with was not consistent with dimensionless types when switching arguments.
String processing was not allowing strings with any magnitude.

- Updated registry.py managing dimensionless types & discarding types not managed by pint.
- Updated unit.py using parse_expression instead of parse_units.
jules-ch added a commit to jules-ch/pint that referenced this issue Oct 16, 2020
Make is_compatible_with function with dimensionless types & string parsing errors.
is_compatible_with was not consistent with dimensionless types when switching arguments.
String processing was not allowing strings with any magnitude.

- Updated registry.py managing dimensionless types & discarding types not managed by pint.
- Updated unit.py using parse_expression instead of parse_units.
jules-ch added a commit to jules-ch/pint that referenced this issue Oct 16, 2020
Make is_compatible_with function with dimensionless types & string parsing errors.
is_compatible_with was not consistent with dimensionless types when switching arguments.
String processing was not allowing strings with any magnitude.

- Updated registry.py managing dimensionless types & discarding types not managed by pint.
- Updated unit.py using parse_expression instead of parse_units.
jules-ch added a commit to jules-ch/pint that referenced this issue Oct 16, 2020
Make is_compatible_with function with dimensionless types & string parsing errors.
is_compatible_with was not consistent with dimensionless types when switching arguments.
String processing was not allowing strings with any magnitude.

- Updated registry.py managing dimensionless types & discarding types not managed by pint.
- Updated unit.py using parse_expression instead of parse_units.
jules-ch added a commit to jules-ch/pint that referenced this issue Oct 28, 2020
…types & string parsing errors.

is_compatible_with was not consistent with dimensionless types when switching arguments.
String processing was not allowing strings with any magnitude.

- Updated registry.py managing dimensionless types & discarding types not managed by pint.
- Updated unit.py using parse_expression instead of parse_units.
jules-ch added a commit to jules-ch/pint that referenced this issue Oct 28, 2020
…types & string parsing errors.

is_compatible_with was not consistent with dimensionless types when switching arguments.
String processing was not allowing strings with any magnitude.

- Updated registry.py managing dimensionless types & discarding types not managed by pint.
- Updated unit.py using parse_expression instead of parse_units.
@hgrecco
Copy link
Owner

hgrecco commented Apr 27, 2023

I think this is fixed, right @jules-ch ?

@jules-ch
Copy link
Collaborator Author

I need to rebase #1191

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants