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

[BugFix] Add json_schema_extra With Choices To Provider 'period' Field #6451

Merged
merged 9 commits into from
May 26, 2024

Conversation

deeleeramone
Copy link
Contributor

  1. Why?:

    • Parameters defined as a Literal do not have a list of choices for the schema.
  2. What?:

    • Defines the Literal choices in json_schema_extra for each provider's period field in balance/cash/income endpoints.
    • Cleans up the "growth" models that are only used by FMP to be more like the balance/cash/income models.
    • Adds json_schema_extra={"x-unit_measurement"} wherever was missing.
  3. Impact:

    • Does not change any params.
    • Pro does not use the growth endpoints, the cleanup here will not impact existing widgets.
    • Improved schema definitions that will help anyone relying on openapi.json or reference.json
    • Rename a couple of Polygon fields to align with others.
  4. Testing Done:

    • Integration and unit tests.
    • 'regular' usage.

@deeleeramone deeleeramone added bug Fix bug platform OpenBB Platform labels May 21, 2024
@hjoaquim hjoaquim added the do not merge Label to prevent pull request merge label May 22, 2024
@hjoaquim
Copy link
Contributor

A lot of changes. Please merge only after release 4.2.1

@hjoaquim hjoaquim removed the do not merge Label to prevent pull request merge label May 23, 2024
filing_date: Optional[date] = Field(
period: Literal["annual", "quarter", "ttm"] = Field(
default="annual",
json_schema_extra={"choices": ["annual", "quarter", "ttm"]},
Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference:
For the CLI to leverage custom choices, one only need to populate the json_schema_extra with them if it's not possible to use such options inside the Literal - cases where we want to leverage comma separated lists and as such the type need to be str instead of Literal.
Here, we're just duplicating the same info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I've experienced, the act of defining almost any Literal is an exercise in duplication, and this appears to be a limitation of Python 3.9, because you cannot dynamically define a Literal.

@hjoaquim hjoaquim added this pull request to the merge queue May 26, 2024
Merged via the queue into develop with commit 5fecd9d May 26, 2024
9 checks passed
@IgorWounds IgorWounds deleted the bugfix/financials-period-choices branch May 27, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking_change bug Fix bug platform OpenBB Platform v4 PRs for v4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants