Skip to content

Conversation

sampan501
Copy link
Contributor

Reference issue

Closes #15009

What does this implement/fix?

The implementation of p-value computation via a permutation test in scipy.stats.multiscale_graphcorr was inconsistent with how they are computed in the literature. This PR fixes that.

Additional information

@sampan501
Copy link
Contributor Author

@mdhaber Just opened a PR

@sampan501
Copy link
Contributor Author

It seems like the failures in the tests are unrelated to my code

Copy link
Member

@tupui tupui 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 @sampan501. Please add a test to verify your changes.

@mdhaber
Copy link
Contributor

mdhaber commented Mar 29, 2022

It would be OK to check that the p-value is divisible by 1/(n+1) but not 1/n, if there's not a more sensitive test.

@sampan501
Copy link
Contributor Author

@tupui added. @mdhaber this test basically does that. In previous tests it returns 1/1000 but this checks if it returns 1/1001

@mdhaber
Copy link
Contributor

mdhaber commented Mar 30, 2022

Thanks @sampan501 @tupui !

@mdhaber mdhaber merged commit 6bc97c4 into scipy:main Mar 30, 2022
@tupui tupui added this to the 1.9.0 milestone Mar 30, 2022
@tupui tupui added defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.stats labels Mar 30, 2022
patnr added a commit to patnr/scipy that referenced this pull request Apr 5, 2022
* master: (632 commits)
  Update _dual_annealing.py (scipy#15939)
  TST: stats: make `check_sample_var` two-sided (scipy#15723)
  DOC: sparse.linalg: add citations for COLAMD
  DOC: fix missing comma in conf
  ENH/MAINT: Version switcher from the sphinx theme (scipy#15380)
  MAINT: stats: remove support for `_rvs` without `size` parameter (scipy#15917)
  BUG: Handle base case for scipy.integrate.simpson when span along the axis is 0  (scipy#15824)
  MAINT: stats: adjust tolerance for failing test only
  MAINT: stats: adjust tolerance of failing TestTruncnorm
  MAINT: special: Clean up C style in ndtr.c
  CI: remove pin on Jinja2 (scipy#15895)
  STY: Remove white spaces
  MAINT: stats: exempt gilbrat from refguide_check
  MAINT: stats: rename continuous_gilbrat->continuous_gibrat
  MAINT: stats: correct name Gilbrat -> Gibrat
  [ENH] circvar calculated simply as 1-R (scipy#5747)
  DEP: deal with deprecation of ndim >1 in bspline
  MAINT: stats: more specific error type from `rv_continuous.fit` (scipy#15778)
  DOC: fix import in example in _morestats (scipy#15900)
  [BUG] make p-values consistent with the literature (scipy#15894)
  ...
@sampan501 sampan501 deleted the patch-1 branch May 5, 2022 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: scipy.stats.multiscale_graphcorr p-values are computed differently from literature and other packages
3 participants