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

[Feature] Update chart creation so it doesn't break the command execution #6382

Merged
merged 3 commits into from
May 9, 2024

Conversation

hjoaquim
Copy link
Contributor

@hjoaquim hjoaquim commented May 9, 2024

  1. Why? (1-3 sentences or a bullet point list):

    • Creating a chart shouldn't break the whole creation of the OBBject (unless in debug mode).
  2. What? (1-3 sentences or a bullet point list):

    • Encapsulate in a try/except block.
  3. Impact (1-2 sentences or a bullet point list):

    • NA / minor UX.
  4. Testing Done:

    • Run any chart - manually insert an exception for testing purposes.

@hjoaquim hjoaquim requested a review from montezdesousa May 9, 2024 10:16
@github-actions github-actions bot added enhancement Enhancement platform OpenBB Platform v4 PRs for v4 labels May 9, 2024
@deeleeramone
Copy link
Contributor

LGTM, on a side-note, which function was failing when chart=True and under what conditions? It would be good to know this so it can be looked into.

@hjoaquim
Copy link
Contributor Author

hjoaquim commented May 9, 2024

LGTM, on a side-note, which function was failing when chart=True and under what conditions? It would be good to know this so it can be looked into.

Actually, any! This come up on a random experiment and just thought it would be a good idea to not break everything just bc the chart couldn't be computed!

@deeleeramone
Copy link
Contributor

Actually, any! This come up on a random experiment and just thought it would be a good idea to not break everything just bc the chart couldn't be computed!

WDYM, any? I understand what this is doing, but am looking for a repeatable scenario where chart=True fails.

@hjoaquim hjoaquim enabled auto-merge May 9, 2024 22:55
@hjoaquim hjoaquim added this pull request to the merge queue May 9, 2024
@hjoaquim
Copy link
Contributor Author

hjoaquim commented May 9, 2024

Actually, any! This come up on a random experiment and just thought it would be a good idea to not break everything just bc the chart couldn't be computed!

WDYM, any? I understand what this is doing, but am looking for a repeatable scenario where chart=True fails.

It does not fail in any function afaik; I forced an exception to check something, not sure about the context anymore tho

Merged via the queue into develop with commit 83476ad May 9, 2024
10 checks passed
@IgorWounds IgorWounds deleted the feature/encapsulate-to-chart branch May 10, 2024 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement platform OpenBB Platform v4 PRs for v4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants