Add FunctionModel for handling functions #1

JoshuaLeivers · 2023-09-04T11:42:07Z

Change Summary

Adds a wrapper model that uses a Pydantic model to validate function arguments and return values, exposing functions to pass in and retrieve arguments, and to call functions using the stored arguments.

Related issue number

This is a feature that has been asked for previously in pydantic#1391 but which was possible through v1 methods which are not present in v2 and so that issue is already closed.

Checklist

The pull request title is a good summary of the changes - it will be used in the changelog
Unit tests for the changes exist
Tests pass on CI
Documentation reflects the changes where applicable
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

a-khabarov · 2023-09-11T16:41:43Z

pydantic/function_model.py

@@ -0,0 +1,442 @@
+"""Provides FunctionModel, a model for validating function arguments."""
+
+from collections import OrderedDict


IIRC all Python dicts are guaranteed to be ordered since Python 3.6 and pydantic 2 supports only Python 3.7+.

Replaced it, good point

pydantic/function_model.py

a-khabarov · 2023-09-11T16:43:13Z

pydantic/function_model.py

+    _keyword_total: int
+    _positional_total: int
+
+    T = TypeVar('T')


Probably best to have this TypeVar outside of the class.

a-khabarov · 2023-09-11T16:46:30Z

pydantic/function_model.py

+
+                params[name] = (
+                    get_annotation_type(param.annotation),
+                    param.default if param.default != Parameter.empty else ...,


I think this won't work if the default value is ....

I got the impression somewhere (possibly here, I'm not sure) that ... was the default value of field defaults. From looking further at that class, it seems they use PydanticUndefined, which I have changed it to and works just as well with the current tests. I can't find any documentation as to what exactly I should use here, but hopefully if there is an issue then it will get pointed out once we have an upstream PR - I'll make sure to ask explicitly when the time comes so it's not missed.

a-khabarov · 2023-09-11T16:47:48Z

pydantic/function_model.py

+        config = (
+            ConfigDict(extra='allow', validate_assignment=True, validate_default=True)
+            if has_kwargs
+            else ConfigDict(extra='forbid', validate_assignment=True, validate_default=True)
+        )


Suggested change

config = (

ConfigDict(extra='allow', validate_assignment=True, validate_default=True)

if has_kwargs

else ConfigDict(extra='forbid', validate_assignment=True, validate_default=True)

)

config = ConfigDict(extra='allow' if has_kwargs else 'forbid', validate_assignment=True, validate_default=True)

a-khabarov · 2023-09-11T16:50:09Z

pydantic/function_model.py

+        if key == 'args':
+            # Values within *args are unrestricted
+            return value
+        elif key == 'kwargs':
+            # Values within **kwargs are unrestricted
+            return value


Suggested change

if key == 'args':

# Values within *args are unrestricted

return value

elif key == 'kwargs':

# Values within **kwargs are unrestricted

return value

if key in {'args', 'kwargs'}:

# Values within *args and **kwargs are unrestricted

return value

Fixed, I believe at some point I had it where the two keys were doing something different and I just forgot to reduce it down.

a-khabarov · 2023-09-11T16:51:51Z

pydantic/function_model.py

+            field = self._model.model_fields.get(key)
+            assert field is not None  # This should always be the case, but Python complains otherwise
+            return TypeAdapter(get_annotation_type(field.annotation)).validate_python(value)


Suggested change

field = self._model.model_fields.get(key)

assert field is not None # This should always be the case, but Python complains otherwise

return TypeAdapter(get_annotation_type(field.annotation)).validate_python(value)

return TypeAdapter(get_annotation_type(self._model.model_fields[key].annotation)).validate_python(value)

?

a-khabarov · 2023-09-11T16:54:29Z

pydantic/function_model.py

+            if pos >= len(param_keys):
+                if has_args:
+                    in_args = True
+                else:
+                    error = unexp_error  # Unknown positional parameter and *args is not present
+            elif not self._parameters[param_keys[pos]]['positional']:
+                if has_args:
+                    in_args = True
+                else:
+                    error = unexp_error  # Keyword-only parameter, so can't set via position


Suggested change

if pos >= len(param_keys):

if has_args:

in_args = True

else:

error = unexp_error # Unknown positional parameter and *args is not present

elif not self._parameters[param_keys[pos]]['positional']:

if has_args:

in_args = True

else:

error = unexp_error # Keyword-only parameter, so can't set via position

if pos >= len(param_keys) or not self._parameters[param_keys[pos]]['positional']:

if has_args:

in_args = True

else:

error = unexp_error

a-khabarov · 2023-09-11T16:55:19Z

pydantic/function_model.py

+                param = params[key]
+                param['set_keyword'] = True if param['keyword'] else False
+
+        if len(errors) > 0:


Suggested change

if len(errors) > 0:

if errors:

a-khabarov · 2023-09-11T16:58:29Z

tests/test_function_model.py

+    pytest.skip('These tests use positional-only parameters in functions, which are not supported prior to Python 3.8')
+
+
+"""def test_only_pos():  # noqa: C901


What is the purpose of this string?

These were just old tests that I had commented out while I was working on replacing them. Forgot to remove them, but have removed them now.

This model type uses a Pydantic model to validate function arguments and return values, exposing functions to pass in and retrieve arguments, and to call functions using the stored arguments. This is a feature that has been asked for previously in pydantic#1391 but which was possible through v1 methods which are not present in v2 and so that issue is already closed.

OrderedDict is unnecessary here, as dicts are ordered by default in all Python versions that Pydantic supports (>=3.7). Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Some old tests were left in the module by mistake within a comment string.

This may be a more appropriate value than the previous, which was Python's ellipsis.

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

The new format functions the same, but is a bit shorter and more readable. Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

`get_signature` allows developers to directly access the function signature of the model, which may be useful in situations where their code has access to the model itself but not the function. Also, `FunctionModel` could not previously be imported via `pydantic.FunctionModel`, which has been fixed.

mattyclarkson · 2023-09-21T15:19:09Z

Function call validation is implemented upstream here. We can close this PR.

JoshuaLeivers added the enhancement New feature or request label Sep 4, 2023

JoshuaLeivers force-pushed the function-model branch 2 times, most recently from eac2e78 to 32ff1ce Compare September 4, 2023 12:44

JoshuaLeivers marked this pull request as ready for review September 4, 2023 12:45

a-khabarov suggested changes Sep 11, 2023

View reviewed changes

JoshuaLeivers force-pushed the function-model branch 4 times, most recently from 96615f7 to a10f210 Compare September 13, 2023 12:28

JoshuaLeivers requested a review from a-khabarov September 13, 2023 13:14

JoshuaLeivers and others added 11 commits September 21, 2023 14:18

Replace OrderedDict with dict in FunctionModel

a2f29ab

OrderedDict is unnecessary here, as dicts are ordered by default in all Python versions that Pydantic supports (>=3.7). Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Remove old tests from test_function_model.py

373b78b

Some old tests were left in the module by mistake within a comment string.

Change FunctionModel field default to PydanticUndefined

c809010

This may be a more appropriate value than the previous, which was Python's ellipsis.

Change wording of FunctionModel docstring

4d5ac81

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Move TypeVar out of FunctionModel

9d1dc27

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Reformat ConfigDict call in FunctionModel

52bc704

The new format functions the same, but is a bit shorter and more readable. Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Fix duplicated if-statement results in FunctionModel

4518ec4

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Remove unnecessary assert in FunctionModel

a5f4929

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Fix duplicated code in FunctionModel argument validation

7367b96

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

Improve readability of error generation in FunctionModel

bfb8b7f

Co-authored-by: Alexander Khabarov <alexander.khabarov@arm.com>

JoshuaLeivers force-pushed the function-model branch from 2ea4e0d to bfb8b7f Compare September 21, 2023 13:19

JoshuaLeivers closed this Sep 21, 2023

tcompa mentioned this pull request Sep 9, 2024

Reduce dependency on pydantic._internal fractal-analytics-platform/fractal-tasks-core#835

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FunctionModel for handling functions #1

Add FunctionModel for handling functions #1

JoshuaLeivers commented Sep 4, 2023 •

edited

Loading

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 12, 2023

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 13, 2023

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 12, 2023 •

edited

Loading

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 13, 2023

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 13, 2023

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 13, 2023

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 13, 2023

a-khabarov Sep 11, 2023

a-khabarov Sep 11, 2023

JoshuaLeivers Sep 13, 2023

mattyclarkson commented Sep 21, 2023

		@@ -0,0 +1,442 @@
		"""Provides FunctionModel, a model for validating function arguments."""

		from collections import OrderedDict

		pytest.skip('These tests use positional-only parameters in functions, which are not supported prior to Python 3.8')


		"""def test_only_pos(): # noqa: C901

Add FunctionModel for handling functions #1

Add FunctionModel for handling functions #1

Conversation

JoshuaLeivers commented Sep 4, 2023 • edited Loading

Change Summary

Related issue number

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshuaLeivers Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattyclarkson commented Sep 21, 2023

JoshuaLeivers commented Sep 4, 2023 •

edited

Loading

JoshuaLeivers Sep 12, 2023 •

edited

Loading