8000 DOC: use notebook-style in plot_prediction_latency.py by ss-is-master-chief · Pull Request #22418 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DOC: use notebook-style in plot_prediction_latency.py # 8000 22418

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 14, 2022
Merged

DOC: use notebook-style in plot_prediction_latency.py #22418

merged 3 commits into from
Feb 14, 2022

Conversation

ss-is-master-chief
Copy link
Contributor

Fixes plot_prediction_latency.py for #22406

lesteve
lesteve reviewed Feb 9, 2022
Copy link
Member
@lesteve lesteve left a comment •
edited
Loading

Choose a reason for hiding this comment

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

OK so I think this example is a bit long and complicated already. Probably it needs to be looked at one day to make it a bit simpler:

For this PR I think I would be fine:

  • adding a header "Benchmark and plot helper functions" before the def atomic_benchmark_estimator function definition
  • remove "main code" that does not bring anything
  • move start_time = time.time() in the "Benchmark bulk/atomic ..." section

@lesteve lesteve mentioned this pull request Feb 9, 2022
Closed
47 tasks
added header suggestions
1010f53 
fix linting error

fix2 linting
@ss-is-master-chief ss-is-master-chief force-pushed the 22406_fix_blocksplitter_plot_prediction_latency branch from c3b53a5 to 1010f53 Compare February 9, 2022 15:00
@ss-is-master-chief
Copy link
Contributor Author

OK so I think this example is a bit long and complicated already. Probably it needs to be looked at one day to make it a bit simpler:

For this PR I think I would be fine:

  • adding a header "Benchmark and plot helper functions" before the def atomic_benchmark_estimator function definition
  • remove "main code" that does not bring anything
  • move start_time = time.time() in the "Benchmark bulk/atomic ..." section

@lesteve Added the changes

8000

@lesteve
Copy link
Member
lesteve commented Feb 9, 2022

Thanks, I think an additional changes which may be nice:

  • split the last cell into 3 cells, one after benchmark, one after plot_n_features, one after plot_benchmark_throughput, at the moment you don't really know which code produces which plot or which output, you get all of them at the end
  • remove the script timing for the total example since sphinx-gallery already does that start_time/stop_time, print(example run in ...)

@lesteve
Copy link
Member
lesteve commented Feb 9, 2022 •
edited
Loading

I think you can use something like this (not 100% sure) to create only code cells:

benchmark(configuration)

## %

# benchmark n_features ...
percentile = 90

plot_n_features_influence(...)

# %%

# benchmark througput

@ss-is-master-chief
Copy link
Contributor Author

@lesteve I have updated the script

@lesteve
Copy link
Member
lesteve commented Feb 14, 2022

Looks good, merging, thanks a lot!

@lesteve lesteve changed the title Fix blocksplitter in plot_prediction_latency.py DOC: use notebook-style in plot_prediction_latency.py Feb 14, 2022
@lesteve lesteve merged commit f083a12 into scikit-learn:main Feb 14, 2022
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Mar 1, 2022
@ss-is-master-chief @thomasjpfan
DOC: use notebook-style in plot_prediction_latency.py (scikit-learn#2…
36b4ebf 
…2418)

Co-authored-by: Sumit Saha <sumitsaha@SumitSahaMacBook.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesteve lesteve lesteve left review comments

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ss-is-master-chief @lesteve
0