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.

Sign up for GitHub

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

Jump to bottom

Add @as_model decorator #268

Merged
merged 10 commits into from
Nov 22, 2023
Merged

Add @as_model decorator #268

merged 10 commits into from
Nov 22, 2023

Conversation

theorashid
Copy link
Contributor

Also move marginal_model into pymc_experimental/model. Hopefully I reorganised the file structure correctly

cc: @ricardoV94 @zaxtax @twiecki

@zaxtax zaxtax requested a review from ricardoV94 November 20, 2023 16:41
@ricardoV94
Copy link
Member

ricardoV94 commented Nov 20, 2023 •
edited
Loading

Just a bit worried about name clash between the module model and the decorator model? What about calling the decorator as_model?

@ricardoV94
Copy link
Member

The URL in the docstrings doesn't seem to render correctly: https://pymcio--268.org.readthedocs.build/projects/experimental/en/268/generated/pymc_experimental.model.html

@ricardoV94 ricardoV94 added the enhancements New feature or request label Nov 20, 2023
@zaxtax
Copy link
Contributor

zaxtax commented Nov 20, 2023

Just a bit worried about name clash between the module model and the decorator model? What about calling the decorator as_model?

Have there been bugs or issues due to this kind of name clash before?

@ricardoV94
Copy link
Member

Have there been bugs or issues due to this kind of name clash before?

We had some issues with docs in the past, when a function had the same name as the module

@theorashid
Copy link
Contributor Author

hopefully all good to go now

@theorashid theorashid changed the title Add @pmx.model decorator Add @pmx.as_model decorator Nov 20, 2023
@twiecki
Copy link
Member

twiecki commented Nov 21, 2023

Would be nice to give @zaxtax some credit here, commit co-author?

@theorashid @zaxtax
update docs to link to zaxtax blog
b5210f7 
Co-authored-by: Rob Zinkov <8529+zaxtax@users.noreply.github.com>
@ricardoV94 ricardoV94 changed the title Add @pmx.as_model decorator Add @as_model decorator Nov 22, 2023
@ricardoV94 ricardoV94 merged commit 8046695 into pymc-devs:main Nov 22, 2023
6 checks passed
@ricardoV94
Copy link
Member

Thanks @theorashid

@theorashid theorashid deleted the add-model-decorator branch November 22, 2023 21:21
ferrine
ferrine reviewed Nov 23, 2023
View reviewed changes
pymc_experimental/model/model_api.py

def decorator(f):
@wraps(f)
def make_model(*args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

I would also add a name kwarg here to change model name

Copy link
Member

Choose a reason for hiding this comment

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

You can pass it to model_kwargs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardoV94 ricardoV94 ricardoV94 approved these changes

@ferrine ferrine ferrine left review comments

@twiecki twiecki twiecki left review comments

Assignees
No one assigned
Labels
enhancements New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@theorashid @ricardoV94 @zaxtax @twiecki @ferrine