Skip to content

Allow creating CustomDist inside another CustomDist #6822

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 1 commit into from
Jul 12, 2023

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Jul 10, 2023

@ricardoV94 ricardoV94 force-pushed the fix_nested_customdist branch from 4866992 to 3b97c29 Compare July 10, 2023 12:49
@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Merging #6822 (4866992) into main (f67ff8b) will decrease coverage by 4.37%.
The diff coverage is 100.00%.

❗ Current head 4866992 differs from pull request most recent head 3b97c29. Consider uploading reports for the commit 3b97c29 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6822      +/-   ##
==========================================
- Coverage   92.02%   87.66%   -4.37%     
==========================================
  Files          95       95              
  Lines       16261    16266       +5     
==========================================
- Hits        14964    14259     -705     
- Misses       1297     2007     +710     
Impacted Files Coverage Δ
pymc/distributions/distribution.py 97.06% <100.00%> (ø)
pymc/model.py 91.00% <100.00%> (+0.05%) ⬆️

... and 7 files with indirect coverage changes

Copy link
Member

@larryshamalama larryshamalama left a comment

Choose a reason for hiding this comment

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

Looks good! A more general question from my side: when do we want to use BlockModelAccess, i.e. prevent user access to Model contexts? Can we specify this in the BlockModelAccess docstring?

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jul 10, 2023

Looks good! A more general question from my side: when do we want to use BlockModelAccess, i.e. prevent user access to Model contexts? Can we specify this in the BlockModelAccess docstring?

I think that's context specific. There may be several reasons to block. The only application so far is in CustomDist to make sure users don't accidentally write pm.Normal("x") instead of pm.Normal.dist() which would otherwise result in a very silent bug: introducing useless variables in the model every time the distribution is (re)created.

@ricardoV94 ricardoV94 merged commit f8581ab into pymc-devs:main Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants