Skip to content
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

Rename moment to support_point #7166

Merged
merged 16 commits into from
Feb 28, 2024

Conversation

aerubanov
Copy link
Contributor

@aerubanov aerubanov commented Feb 21, 2024

Description

Rename moment method to avoid confusion with distribution mean.

Related Issue

Checklist

  • Rename moment method in code (and other related variable names)
  • Update documentation

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7166.org.readthedocs.build/en/7166/

@aerubanov aerubanov marked this pull request as draft February 21, 2024 15:02
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

Attention: Patch coverage is 96.39175% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 92.26%. Comparing base (74748c7) to head (32f3fd2).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7166      +/-   ##
==========================================
- Coverage   92.29%   92.26%   -0.04%     
==========================================
  Files         101      101              
  Lines       16947    16966      +19     
==========================================
+ Hits        15642    15653      +11     
- Misses       1305     1313       +8     
Files Coverage Δ
pymc/distributions/censored.py 100.00% <100.00%> (ø)
pymc/distributions/continuous.py 97.53% <100.00%> (ø)
pymc/distributions/discrete.py 99.10% <100.00%> (ø)
pymc/distributions/mixture.py 94.95% <100.00%> (ø)
pymc/distributions/multivariate.py 93.78% <100.00%> (ø)
pymc/distributions/simulator.py 84.17% <100.00%> (ø)
pymc/distributions/timeseries.py 94.45% <100.00%> (ø)
pymc/distributions/truncated.py 99.40% <100.00%> (ø)
pymc/initial_point.py 100.00% <100.00%> (ø)
pymc/testing.py 91.10% <80.00%> (-0.39%) ⬇️
... and 1 more

@aerubanov aerubanov force-pushed the rename-moment-method branch from 3bb5b5b to 4813f2d Compare February 23, 2024 07:56
@aerubanov aerubanov marked this pull request as ready for review February 23, 2024 08:39
@aerubanov
Copy link
Contributor Author

aerubanov commented Feb 23, 2024

I renamed moment to finite_logp_point in code. Will change docs accordingly next step.

@aerubanov
Copy link
Contributor Author

@ricardoV94 could you please take a look?

@ricardoV94
Copy link
Member

@aerubanov great work. I have some suggestions, let me know what you think:

  1. Instead of finite_logp_point, call it support_point? A bit less verbose and still precise
  2. Don't break code for people defining Distributions with moment methods or using the old _moment dispatch or the user-helper moment. Instead issue an informative FutureWarning about the name change.

@ricardoV94 ricardoV94 added maintenance major Include in major changes release notes section labels Feb 27, 2024
@aerubanov
Copy link
Contributor Author

@ricardoV94 your suggestions sounds good, will implement it

@@ -653,34 +653,34 @@ def check_selfconsistency_discrete_logcdf(
)


def assert_moment_is_expected(model, expected, check_finite_logp=True):
def assert_support_point_is_expected(model, expected, check_finite_logp=True):
Copy link
Member

Choose a reason for hiding this comment

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

This is also user facing (in that we use this utility in other libraries), so should also add a wrapper with a deprecation warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added option for using old name

@ricardoV94 ricardoV94 merged commit ecbcf38 into pymc-devs:main Feb 28, 2024
23 checks passed
@aerubanov aerubanov deleted the rename-moment-method branch February 28, 2024 11:27
@ricardoV94 ricardoV94 changed the title Rename moment to finite_logp_point Rename moment to support_point Mar 15, 2024
mkusnetsov pushed a commit to mkusnetsov/pymc that referenced this pull request Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance major Include in major changes release notes section
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename moment to finite_logp_point
2 participants