Skip to content

Refactor (mostly rearrange) the statistics module #119930

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 13 commits into from
Jun 2, 2024

Conversation

rhettinger
Copy link
Contributor

  • Move duplicated code out of kde() and kde_random() into a shared dictionary.
  • Move kernel specifications into separate functions for better maintainability.
  • Add clearer section dividers and move sections into a logical order.
  • Put the private utilities at the bottom.
  • Add line breaks to improve readability.
  • Update a "fixme" comment.

@rhettinger rhettinger merged commit e378dc1 into python:main Jun 2, 2024
31 checks passed
@rhettinger rhettinger deleted the kde_refactor branch June 2, 2024 03:07
@zware
Copy link
Member

zware commented Jun 2, 2024

Just to note, this is the kind of change that if anyone else on Earth had made this PR, it would have been rejected out of hand. The entire module is rewritten, with no review or even discussion. It was merged within 1 hour of posting, late at night for most contributors.

Realistically, the change is probably fine and there probably aren't any backdoors hidden in 1800 lines of changes, because Raymond probably retains full control of his account and is a trusted contributor. However, I do not believe this should pass without comment and until someone can actually review this change, I would be a lot more comfortable if it were reverted.

@zware
Copy link
Member

zware commented Jun 3, 2024

This has been nagging at me enough that I have looked through it myself and find no evidence of foul play, but it's also impossible to tell what if anything actually changed and in a change this size (even looking at individual commits) I could very easily have missed something. From a security viewpoint, this PR is terrifying and is strong evidence that we should revisit our policy of not requiring reviews regardless of the major inconvenience that poses.

(cc @hugovk as 3.14 RM)

barneygale pushed a commit to barneygale/cpython that referenced this pull request Jun 5, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
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.

2 participants