- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.3k
bpo-45876: Correctly rounded stdev() and pstdev() for the Decimal case #29828
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
                    Changes from all commits
      Commits
    
    
            Show all changes
          
          
            37 commits
          
        
        Select commit
          Hold shift + click to select a range
      
      bbd2da9
              
                Merge pull request #1 from python/master
              
              
                rhettinger 74bdf1b
              
                Merge branch 'master' of github.com:python/cpython
              
              
                rhettinger 6c53f1a
              
                Merge branch 'master' of github.com:python/cpython
              
              
                rhettinger a487c4f
              
                .
              
              
                rhettinger eb56423
              
                .
              
              
                rhettinger cc7ba06
              
                .
              
              
                rhettinger d024dd0
              
                .
              
              
                rhettinger b10f912
              
                merge
              
              
                rhettinger fb6744d
              
                merge
              
              
                rhettinger 7f21a1c
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger 7da42d4
              
                Merge branch 'main' of github.com:rhettinger/cpython
              
              
                rhettinger e31757b
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger f058a6f
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger 1fc29bd
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger e5c0184
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger 3c86ec1
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger 96675e4
              
                Merge branch 'main' of github.com:rhettinger/cpython
              
              
                rhettinger de558c6
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger 418a07f
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger ea23a8b
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger ba248b7
              
                Merge branch 'main' of github.com:python/cpython
              
              
                rhettinger 037b5fe
              
                Correctly rounded stdev results for Decimal inputs
              
              
                rhettinger f5c091c
              
                Whitespace
              
              
                rhettinger 70cdade
              
                Rename the functions consistently
              
              
                rhettinger 82dbec6
              
                Improve comment
              
              
                rhettinger 1a6c58d
              
                Tweak variable names
              
              
                rhettinger b2385b0
              
                Replace Fraction arithmetic with integer arithmetic
              
              
                rhettinger 594ea27
              
                Add spacing between terms
              
              
                rhettinger 3911581
              
                Fix type annotation
              
              
                rhettinger a09e3c4
              
                Return a Decimal zero when the numerator is zero
              
              
                rhettinger 152ed3f
              
                Remove unused import
              
              
                rhettinger 80371c1
              
                Factor lhs of inequality. Rename helper function for consistency.
              
              
                rhettinger 1c86e7c
              
                Add comment for future work.
              
              
                rhettinger 0684fac
              
                Fix typo in docstring. Refine wording in comment.
              
              
                rhettinger 8b5e377
              
                Add more detail to the comment about numerator and denominator sizes
              
              
                rhettinger d11d567
              
                Improve variable name
              
              
                rhettinger 309cb0a
              
                Avoid double rounding in test code
              
              
                rhettinger File filter
Filter by extension
Conversations
          Failed to load comments.   
        
        
          
      Loading
        
  Jump to
        
          Jump to file
        
      
      
          Failed to load files.   
        
        
          
      Loading
        
  Diff view
Diff view
There are no files selected for viewing
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be checking
mhere rather thann? It looks as though we multiply through bymbelow, so we needmto be positive for the inequalities to work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The case where m is zero is handled before the inequality test. The
Decimal(n) / Decimal(m)step raises DivisionByZero which is a subclass of ZeroDivisionError. There is a test for this case.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was really more worried about the case m negative, not m zero. It seems more natural to have the normalization step
n, m = -n, -mensure that the denominator is positive than that the numerator is positive: right now, we're effectively converting-2 / 5to2 / (-5), and what we care about below for the maths to work is thatmis positive, not thatnis positive: you're implicitly converting the inequalityn / m > ((root + plus) / 2) ** 2to the inequalityn > ((root + plus) / 2)**2 * m, and that conversion is only valid ifmis positive.It doesn't matter, because if just one of
nandmis negative then the(Decimal(n) / Decimal(m)).sqrt()step will fail (except in the case of really extrememwhere the division rounds from something tiny and negative to negative zero); it just seems like a surprising choice and for me at least it made the code harder to read and reason about.