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/econometrics: Added Variance Inflation Factor #5866

Merged
merged 16 commits into from
Nov 25, 2024

Conversation

HemuManju
Copy link
Contributor

@HemuManju HemuManju commented Dec 6, 2023

Description

Hello Everyone,

I hope this message finds you well.

In this pull request, I have included the following models for econometrics:

  • Variance Inflation Factor (VIF)

How has this been tested?

  • I have followed the test structure, like the technical analysis (ta) extension.
  • All the tests (API and Python) are included in the integration folder.
  • Make sure affected commands still run in terminal
  • Ensure the SDK still works
  • Check any related reports

Checklist:

Others

  • I have performed a self-review of my own code.
  • I have commented on my code, particularly in hard-to-understand areas.

⚠️ Disclaimer: This is a rewrite of the original openbb_terminal feature in v4 SDK. Credit goes to its original authors.

@reviewpad reviewpad bot added the feat XS Extra small feature label Dec 6, 2023

# Calculate the VIF values
vif_values = pd.DataFrame(
{"VIF Values": [vif(df.values, i) for i in range(df.shape[1])][1:]},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This actually results in the dataframe cell having the text "VIF Values:" Would be beneficial to be able to use the returned value without having to process it.

@jmaslek
Copy link
Collaborator

jmaslek commented Dec 7, 2023

Thanks for bringing this one over! I left a comment in the code, but it would be nice to be able to associate a variable with the vif in the output, so returning the column as well in the df would be my initial thoughts...

@HemuManju
Copy link
Contributor Author

@jmaslek If I understand correctly, the output should be something similar to f{column_name}_vif_value instead of just returning a list of vif values?

@jmaslek
Copy link
Collaborator

jmaslek commented Dec 7, 2023

I think we can return a dataframe where the columns are the columns of the input and the values are the corresponding big value.

@HemuManju
Copy link
Contributor Author

HemuManju commented Dec 7, 2023

Now the result is a dataframe with columns from input and values are VIF values. Also changed the implementation slightly to improve readability.

Copy link
Collaborator

@jmaslek jmaslek left a comment

Choose a reason for hiding this comment

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

Sorry for the delay but this looks good o my end!

@deeleeramone deeleeramone added this pull request to the merge queue Nov 25, 2024
Merged via the queue into OpenBB-finance:develop with commit 6fa993d Nov 25, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat XS Extra small feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants