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

add: M&A impact example notebook #6893

Merged
merged 7 commits into from
Oct 30, 2024
Merged

Conversation

NabidAkhtar
Copy link
Contributor

@NabidAkhtar NabidAkhtar commented Oct 27, 2024

Pull Request the OpenBB Platform

Description

  • Summary of the change/ bug fix: Added a new Jupyter notebook example demonstrating how to perform M&A impact analysis using OpenBB's historical data functions. This notebook analyzes stock performance of companies around M&A announcements, with visualizations and a summary report.
  • Link [🕹️] Design a Notebook for Evaluating Mergers & Acquisitions #6714 , if applicable.
  • Screenshot of the feature or the bug before/after fix, if applicable:
    image
  • Relevant motivation and context: This example notebook provides OpenBB users with a practical demonstration of M&A impact analysis, showcasing OpenBB’s data retrieval and analysis capabilities.
  • List any dependencies that are required for this change: Added matplotlib to the installation cell to ensure compatibility for users outside of Colab, where it may not be pre-installed.

How has this been tested?

  • Please describe the tests that you ran to verify your changes: The notebook was run in a Jupyter environment to confirm that all cells execute as expected and produce correct outputs.
  • Please provide instructions so we can reproduce: To reproduce, run the notebook in a Jupyter environment with OpenBB installed.
  • Please also list any relevant details for your test configuration: Verified data retrieval, accurate calculations, and expected visualizations.

Commands Execution Verification: Not applicable for this notebook addition.

  • Ensure all unit and integration tests pass.
  • If you modified/added command(s):
    • Ensure the command(s) execute with the expected output.
      • API.
      • Python Interface.
    • If applicable, please add new tests for the command (see CONTRIBUTING.md to leverage semi-automated testing).
  • If a new provider was introduced or a new fetcher was added to an existing provider:
    • Ensure the existing tests pass.
    • Ensure the new provider and/or fetcher is stable and usable.
    • If applicable, please add new tests for the provider and/or fetcher (see CONTRIBUTING.md to leverage semi-automated testing).
  • If a new provider or extension was added:

Checklist

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have adhered to the GitFlow naming convention and my branch name is in the format of feature/feature-name or hotfix/hotfix-name.
  • I ensure that I am following the CONTRIBUTING guidelines.

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2024

CLA assistant check
All committers have signed the CLA.

@NabidAkhtar NabidAkhtar changed the title Add M&A impact example notebook add: M&A impact example notebook Oct 27, 2024
@piiq
Copy link
Contributor

piiq commented Oct 28, 2024

resolves #6714

@piiq
Copy link
Contributor

piiq commented Oct 28, 2024

Hi, 3 suggestions

  1. The openbb package does not bring in matplotlib and seaborn so you will need to update the installation instruction cell to specify that matplotlib and seaborn (?) need to be installed for this notebook to run. It currently works on colab because google pre-installes both of these packages into the default colab environment.
  2. When providing a link for the "run in colab" badge - please reference the notebook in the repo, not in your personal google drive
  3. seaborn is not used in the notebook so it would make sense to remove the import and remove the mention of the requirement to install it

Thanks

@NabidAkhtar
Copy link
Contributor Author

@piiq I’ve implemented all suggested changes:
Added matplotlib in the installation instructions to ensure compatibility across environments.
Updated the Colab link to reference the repository location directly.
Removed the unused seaborn import.
Thank you for the feedback! Please review when you have a chance.

@piiq
Copy link
Contributor

piiq commented Oct 28, 2024

Thanks 2 more suggestions

  1. In the repo we have the the convention to use "symbol" instead of "ticker". To make it more aligned please consider also using "symbol"
  2. In the report and the chart you can use the actual symbol instead of "ACQUIRER_TICKER" and "TARGET_TICKER" strings and the actual announcement date instead of "ANNOUNCEMENT_DATE" string

@piiq
Copy link
Contributor

piiq commented Oct 28, 2024

Once this is addressed I am happy to approve and merge this notebook
Thanks

@NabidAkhtar
Copy link
Contributor Author

@piiq I’ve implemented the additional changes:

  • Replaced "ticker" with "symbol" throughout the notebook for consistency with repo conventions.
  • Updated plot_ma_analysis and generate_ma_report to display the actual symbols and announcement date instead of placeholders.

Please review the changes at your convenience. Thanks for the guidance!

@piiq
Copy link
Contributor

piiq commented Oct 30, 2024

/award 300

Copy link

oss-gg bot commented Oct 30, 2024

Awarding NabidAkhtar: 300 points 🕹️ Well done! Check out your new contribution on oss.gg/NabidAkhtar

@piiq piiq enabled auto-merge October 30, 2024 12:22
@piiq piiq added this pull request to the merge queue Oct 30, 2024
Merged via the queue into OpenBB-finance:develop with commit a958bf2 Oct 30, 2024
6 checks passed
Copy link

oss-gg bot commented Oct 30, 2024

Awarding NabidAkhtar: 300 points 🕹️ Well done! Check out your new contribution on oss.gg/NabidAkhtar

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

Successfully merging this pull request may close these issues.

4 participants