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

Update decision tree ensemble notebook to not use exec #1

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

teastburn
Copy link

@teastburn teastburn commented Dec 23, 2023

Why

I appreciated the youtube series and wanted to give back with my expertise!

What

This code doesn't need exec, and exec and eval are considered anti-patterns, with some security implications. Particularly in a shared repo where the assumption is "trust me to run this code".

https://stackoverflow.com/questions/1933451/why-should-exec-and-eval-be-avoided

Tested

I tested it by running it in a Jupyter notebook in Pycharm and verifying the outputs by my eye. Certainly not the most foolproof testing method.

I appreciated the youtube series and wanted to give back with my expertise!

This code doesn't need exec, and exec and eval are considered anti-patterns, with some security implications.

https://stackoverflow.com/questions/1933451/why-should-exec-and-eval-be-avoided
"AdaBoostClassifier 0.956 0.970 0.963 \n",
"GradientBoostingClassifier 0.970 0.970 0.970 "
]
"text/plain": " precision_train recall_train f1_train \\\nDecisionTreeClassifier 1.0 1.0 1.0 \nRandomForestClassifier 1.0 1.0 1.0 \nAdaBoostClassifier 1.0 1.0 1.0 \nGradientBoostingClassifier 1.0 1.0 1.0 \n\n precision_test recall_test f1_test \nDecisionTreeClassifier 0.952 0.896 0.923 \nRandomForestClassifier 0.985 0.955 0.970 \nAdaBoostClassifier 0.956 0.970 0.963 \nGradientBoostingClassifier 0.970 0.970 0.970 ",
Copy link
Author

@teastburn teastburn Dec 23, 2023

Choose a reason for hiding this comment

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

i'm not sure why my pycharm/jupyter setup reformatted this. the previous formatting was more readable. but i wanted to be sure the correctness of the output was the same, so i included it here.

@ShawhinT
Copy link
Owner

Thanks for refactoring this @teastburn! Since the video and blog review the original example code, I want to preserve it.

However, your version is also helpful. Would you mind making another request where your version is a separate file named "tree_ensemble_example-no_exec.ipynb"?

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.

2 participants