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

Fix for issue #7563 #7573

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix for issue #7563 #7573

wants to merge 8 commits into from

Conversation

tanishy7777
Copy link

@tanishy7777 tanishy7777 commented Nov 13, 2024

Replaced "log-likelihood" with "distribution" in for all the classes where distributions is more appropriate rather than log-likelihood.

Description

Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality, as the class provides more than just log-likelihood, including mean, variance and covariance.

Related Issue

Checklist

Type of change

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

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

Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality, as the class provides more than just log-likelihood, including cdf, mean, and random methods.
Copy link

welcome bot commented Nov 13, 2024

Thank You Banner]
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

@ricardoV94
Copy link
Member

Hi @tanishy7777 the idea is to change all the cases where this happened, not just MvNormal

Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality, as the class provides more than just log-likelihood, including mean, variance, covariance.
@tanishy7777
Copy link
Author

I've implemented the changes as per your suggestions in the latest commit. Apologies for the delayed response my end-semester exams were going on. @ricardoV94

@ricardoV94
Copy link
Member

Does this happen only in multivariate.py?

Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality, as the class provides more than just log-likelihood, including mean, variance, covariance, etc.
Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality, as the class provides more than just log-likelihood, including mean, variance, covariance.
Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality, as the class provides more than just log-likelihood including log-cdf, log-pdf.
@tanishy7777
Copy link
Author

tanishy7777 commented Nov 27, 2024

Oh ok, will make the necessary changes. I think only the pymc/distributions folder has the files where the changes need to be made right?
Also I am not sure why some tests are failing for continous.py and discrete.py

Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality, as the class provides more than just log-likelihood, including mean, variance and covariance.
Updated the docstrings for better clarity. Replaced "log-likelihood" with "distribution" to accurately describe the functionality.
@tanishy7777
Copy link
Author

tanishy7777 commented Nov 28, 2024

I have updated all the files that need to be changes in the pymc/distributions folder. Please let me know if any other changes need to be made or if anything needs to be modified @ricardoV94 @twiecki

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.

Docstrings of distributions should not read x log-likelihood
2 participants