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

added isinstance checks to fix tox pyright errors #39780

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Release History

## 1.0.0b3 (Unreleased)

### Bugs Fixed

Fixed tox pyright errors present in the build (Argument of type "Unknown" and Cannot access attribute)

## 1.0.0b2 (Unreleased)

### Features Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,15 +323,27 @@ def _get_type_alias_type(module_name: str, alias_name: str):
return types[alias_name]


def _get_model(module_name: str, model_name: str):
def _get_model(module_name: str, model_name: typing.Union[str, typing.ForwardRef]) -> typing.Any:
Copy link
Member

Choose a reason for hiding this comment

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

Given these typing errors arise from the generated code, it is up to the code generator to fix them. We test our code generator against the latest versions of the type checkers we run, so it should generate type clean code for each client library. It looks like this library was last generated ~7 months ago. I suspect if we regenerate the code with the latest version of the generator that these errors will be cleaned up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @kristapratico for your comment. It prompted me to review the documentation again. I found it odd that I needed to modify these files, as the comments didn't indicate they were auto-generated.

I now understand that these files need to be rebuilt locally using the tsp-client and the latest azure-rest-api-specs for our project. As you mentioned, this hasn't been done in the last ~7 months, and I just picked this up. Apologies for the confusion.

Copy link
Member Author

@chrisribe chrisribe Feb 24, 2025

Choose a reason for hiding this comment

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

Ok seems I am stepping on toes here and a PR is already in review:
#38437

Dropping this PR request.
Sorry for the bother.

models = {k: v for k, v in sys.modules[module_name].__dict__.items() if isinstance(v, type)}
module_end = module_name.rsplit(".", 1)[0]
models.update({k: v for k, v in sys.modules[module_end].__dict__.items() if isinstance(v, type)})
if isinstance(model_name, str):
model_name = model_name.split(".")[-1]
if model_name not in models:
return model_name
return models[model_name]

# Get the actual name string from the ForwardRef if that's what we received
if isinstance(model_name, typing.ForwardRef):
name_str = typing.cast(str, getattr(model_name, "__forward_arg__", ""))
else:
name_str = typing.cast(str, model_name) if model_name else ""

if isinstance(name_str, str):
name_str = name_str.split(".")[-1]

# Look up the model class by name
if not isinstance(name_str, str):
raise ValueError("Model name must be a string or ForwardRef")

if name_str not in models:
return name_str
return models[name_str]


_UNSET = object()
Expand Down Expand Up @@ -432,7 +444,7 @@ def _serialize(o, format: typing.Optional[str] = None): # pylint: disable=too-m
if isinstance(o, dict):
return {k: _serialize(v, format) for k, v in o.items()}
if isinstance(o, set):
return {_serialize(x, format) for x in o}
return list(_serialize(x, format) for x in o) # Convert to list to avoid hashability issues
if isinstance(o, tuple):
return tuple(_serialize(x, format) for x in o)
if isinstance(o, (bytes, bytearray)):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1585,10 +1585,20 @@ def _instantiate_model(self, response, attrs, additional_properties=None):
if callable(response):
subtype = getattr(response, "_subtype_map", {})
try:
readonly = [k for k, v in response._validation.items() if v.get("readonly")]
const = [k for k, v in response._validation.items() if v.get("constant")]
if isinstance(response, type):
readonly = [k for k, v in response._validation.items() if v.get("readonly")]
else:
readonly = []
if isinstance(response, type):
const = [k for k, v in response._validation.items() if v.get("constant")]
else:
const = []
kwargs = {k: v for k, v in attrs.items() if k not in subtype and k not in readonly + const}
response_obj = response(**kwargs)
if isinstance(response, type) and issubclass(response, Model):
response_obj = response(**kwargs)
else:
raise TypeError("Response is not a subclass of Model")

for attr in readonly:
setattr(response_obj, attr, attrs.get(attr))
if additional_properties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
# Changes may cause incorrect behavior and will be lost if the code is regenerated.
# --------------------------------------------------------------------------

VERSION = "1.0.0b2"
VERSION = "1.0.0b3"