-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[BugFix] Fix ReferenceGenerator Unions and Choices #6599
Changes from all commits
41f7899
7981965
9b90242
6451fe0
aae29e9
f23734f
9fb8a60
9e39a60
b955c5f
91ec303
2122f20
54f794e
d2c1399
6469589
dc01be9
172f086
16a2974
c2f5959
06310bb
9842809
58f6158
d440058
f2e153b
2dd194d
0c03f9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,6 @@ class BalanceSheetQueryParams(QueryParams): | |
"""Balance Sheet Query.""" | ||
|
||
symbol: str = Field(description=QUERY_DESCRIPTIONS.get("symbol", "")) | ||
period: str = Field( | ||
default="annual", | ||
description=QUERY_DESCRIPTIONS.get("period", ""), | ||
) | ||
Comment on lines
-19
to
-22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we removing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this is part of the issue, docstrings and type hints do not generate correctly. Long-standing issue being resolved. |
||
limit: Optional[NonNegativeInt] = Field( | ||
default=5, description=QUERY_DESCRIPTIONS.get("limit", "") | ||
) | ||
|
@@ -30,12 +26,6 @@ def to_upper(cls, v: str): | |
"""Convert field to uppercase.""" | ||
return v.upper() | ||
|
||
@field_validator("period", mode="before", check_fields=False) | ||
@classmethod | ||
def to_lower(cls, v: Optional[str]) -> Optional[str]: | ||
"""Convert field to lowercase.""" | ||
return v.lower() if v else v | ||
|
||
|
||
class BalanceSheetData(Data): | ||
"""Balance Sheet Data.""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable name is misleading as it doesn't take only the choices but the extra info (being it choices and multiple items allowed info)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a local variable for iterating over choices for each the provider. The only thing ever assigned to it are choices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it? line 262 is
choices[p] = {"multiple_items_allowed": True, "choices": v.get("choices")}
sry if i'm misinterpreting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a dictionary of choices for the field, by provider, where "multiple_items_allowed" is part of "choices".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also find this variable name ambiguous here because
choices
is also used with a different meaning as a key:value pair inside the dictionary.I suggest
json_schema_extra
, since it will be used as input for that argument later