Skip to content

Conversation

@tadelesh
Copy link
Member

@tadelesh tadelesh commented Jul 7, 2023

@tadelesh tadelesh force-pushed the add_as_dict_support branch from 8c5698a to 87db614 Compare July 25, 2023 11:58
@tadelesh tadelesh force-pushed the add_as_dict_support branch from 87db614 to ca18350 Compare July 27, 2023 09:21
k: v
for k, v in sys.modules[module_name].__dict__.items()
if isinstance(v, type)
if isinstance(v, type) or isinstance(v, typing._GenericAlias)
Copy link
Member Author

Choose a reason for hiding this comment

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

This could resolve type alias problem.

Comment on lines +591 to +598
# is it a forward ref / in quotes?
if isinstance(annotation, (str, typing.ForwardRef)):
try:
model_name = annotation.__forward_arg__ # type: ignore
except AttributeError:
model_name = annotation
if module is not None:
annotation = _get_model(module, model_name)
Copy link
Member Author

Choose a reason for hiding this comment

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

ForwardRef should be resolved at first.

Comment on lines 648 to 658
if getattr(annotation, "__origin__", None) is typing.Union:

def _deserialize_with_union(union_annotation, obj):
for t in union_annotation.__args__:
try:
return _deserialize(t, obj, module, rf)
except DeserializationError:
pass
raise DeserializationError()

return functools.partial(_deserialize_with_union, annotation)
Copy link
Member Author

Choose a reason for hiding this comment

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

Union should be handled after Optional, bc Optional is also a Union.

@tadelesh tadelesh marked this pull request as ready for review July 27, 2023 09:31
return cls(data)
return mapped_cls._deserialize(data) # pylint: disable=protected-access

def as_dict(self, *, exclude_readonly: bool = False, exclude_none: bool = False) -> typing.Dict[str, typing.Any]:
Copy link
Member Author

Choose a reason for hiding this comment

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

json.dumps will return a string and need to loads again to get dict. I prefer to use more efficient way.

@tadelesh tadelesh force-pushed the add_as_dict_support branch from 88527e5 to f5a67e2 Compare July 31, 2023 09:59
@tadelesh tadelesh force-pushed the add_as_dict_support branch 2 times, most recently from 6f1df72 to 9d9e3f6 Compare August 2, 2023 06:33
@tadelesh tadelesh force-pushed the add_as_dict_support branch from 9d9e3f6 to 5f1ddcd Compare August 2, 2023 06:53
p._rest_name for p in o._attr_to_rest_field.values() if _is_readonly(p)
]
return {k: v for k, v in o.items() if k not in readonly_props}
result = dict(o.items())
Copy link
Member

Choose a reason for hiding this comment

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

Why are we creating the dict and then pop:ping read-only properties? This seems less efficient than filtering them out before adding them to the new dict....

Copy link
Member Author

@tadelesh tadelesh Aug 3, 2023

Choose a reason for hiding this comment

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

Oh, the logic is for easily deal with exclude_none in the same time. Since we decide to add exclude_none later, I've refined the code.

k: v
for k, v in sys.modules[module_name].__dict__.items()
if isinstance(v, type)
if isinstance(v, (type, typing._GenericAlias)) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

This looks really odd. What are we trying to accomplish?

Copy link
Member Author

@tadelesh tadelesh Aug 3, 2023

Choose a reason for hiding this comment

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

Actually I want to resolve TypeAlias as follows.

MyNamedUnion = Union["UnionModel1", "UnionModel2"]

class ModelWithNamedUnionProperty(Model):
    named_union: "MyNamedUnion" = rest_field(name="namedUnion")

I've changed the logic a little to:

# is it a type alias?
if isinstance(annotation, str): # type: ignore
if module is not None:
annotation = _get_type_alias_type(module, annotation)

@tadelesh tadelesh force-pushed the add_as_dict_support branch from a2e5857 to ff8f377 Compare August 3, 2023 09:22
@tadelesh
Copy link
Member Author

close bc all the code and test has been migrated to generator, refer this PR: Azure/autorest.python#2027

@tadelesh tadelesh closed this Aug 10, 2023
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.

5 participants