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

bug fix for spark chart #483

Merged
merged 1 commit into from
Jul 5, 2024
Merged

bug fix for spark chart #483

merged 1 commit into from
Jul 5, 2024

Conversation

grzesir
Copy link
Contributor

@grzesir grzesir commented Jul 5, 2024

No description provided.

Copy link
Contributor

korbit-ai bot commented Jul 5, 2024

My review is in progress 📖 - I will have feedback for you in a few minutes!

@grzesir grzesir merged commit c938984 into dev Jul 5, 2024
1 check passed
Copy link
Contributor

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

I have reviewed your code and found 2 potential issues. To discuss my individual comments that I have added, tag me in replies using @korbit-ai.


Please react with a 👍 to my comments that you find helpful and a 👎 to those you find unhelpful - this will help me learn and improve as we collaborate.

@@ -5,7 +5,7 @@

setuptools.setup(
name="lumibot",
version="3.5.11",
version="3.5.12",
Copy link
Contributor

Choose a reason for hiding this comment

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

category Documentation

I noticed that the version of the library was updated in the 'setup.py' file, but there is no documentation explaining what changes were made in this version. It would be helpful to add a 'CHANGELOG.md' file to the repository or update it if it already exists. This file should contain a record of all changes made in each version of the library. For this specific pull request, a new entry should be added for version 3.5.12, explaining that a bug in the spark chart feature was fixed.

@@ -3904,6 +3904,9 @@ def send_spark_chart_to_discord(self, stats_df, portfolio_value, now, days=180):
# Resanple the stats dataframe to daily but keep the datetime column
stats_df = stats_df.resample("D", on="datetime").last().reset_index()

# Drop the cash column because it's not needed
stats_df = stats_df.drop(columns=["cash"])
Copy link
Contributor

Choose a reason for hiding this comment

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

category Objects and Data Structures

It seems that the 'cash' column is being dropped without checking if it exists in the dataframe. This could lead to a KeyError if the column is not present. To resolve this, you should check if the 'cash' column exists before attempting to drop it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant