8000 DOC Documents fixed to increase speed by removing for loop by narendrasinghdangi · Pull Request #27066 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC Documents fixed to increase speed by removing for loop #27066

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

Merged
merged 4 commits into from
Aug 15, 2023

Conversation

narendrasinghdangi
Copy link
Contributor
@narendrasinghdangi narendrasinghdangi commented Aug 14, 2023

Reference Issues/PRs

Closes #27065

What does this implement/fix? Explain your changes.

Increase the speed by removing for loop O(n) -> O(1)

Any other comments?

@github-actions
Copy link
github-actions bot commented Aug 14, 2023

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 3e3cd1d. Link to the linter CI: here

Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @narendrasinghdangi ! I am okay with the proposed change.

@@ -232,7 +232,7 @@ def plot_n_features_influence(percentiles, percentile):
fig, ax1 = plt.subplots(figsize=(10, 6))
colors = ["r", "g", "b"]
for i, cls_name in enumerate(percentiles.keys()):
x = np.array(sorted([n for n in percentiles[cls_name].keys()]))
x = np.array(sorted(list(percentiles[cls_name].keys())))
Copy link
Member

Choose a reason for hiding this comment

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

In this case, I do not think the list is required either:

Suggested change
x = np.array(sorted(list(percentiles[cls_name].keys())))
x = np.array(sorted(percentiles[cls_name].keys()))

Copy link
Contributor Author
@narendrasinghdangi narendrasinghdangi Aug 14, 2023

Choose a reason for hiding this comment

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

yes, you are right i have changed the file now you can merge now to main branch
thank you

@thomasjpfan thomasjpfan changed the title Documents fixed to increase speed by removing for loop DOC Documents fixed to increase speed by removing for loop Aug 14, 2023
@narendrasinghdangi
Copy link
Contributor Author

Thank you for your suggestion i have changed the file now.

Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasjpfan thomasjpfan enabled auto-merge (squash) August 15, 2023 14:33
@thomasjpfan thomasjpfan merged commit 06a6b93 into scikit-learn:main Aug 15, 2023
TamaraAtanasoska pushed a commit to TamaraAtanasoska/scikit-learn that referenced this pull request Aug 21, 2023
akaashpatelmns pushed a commit to akaashp2000/scikit-learn that referenced this pull request Aug 25, 2023
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
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.

Documents fixed to increase speed by removing for loop
2 participants
0