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

Refactor ModelGraph for v4 #4818

Merged
merged 3 commits into from
Jun 29, 2021
Merged

Conversation

michaelosthege
Copy link
Member

@michaelosthege michaelosthege commented Jun 28, 2021

Closes #4816

Thank your for opening a PR!

Depending on what your PR does, here are a few things you might want to address in the description:

  • what are the (breaking) changes that this PR makes? None
  • important background, or details about the implementation does not apply
  • are the changes—especially new features—covered by tests and docstrings? yup
  • linting/style checks have been run yup
  • consider adding/updating relevant example notebooks
  • right before it's ready to merge, mention the PR in the RELEASE-NOTES.md Not relevant, because it's just preventing API breakage.

@michaelosthege michaelosthege marked this pull request as ready for review June 28, 2021 19:46
@michaelosthege michaelosthege self-assigned this Jun 28, 2021
Copy link
Member

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

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

Great to have this functionality back. Should we open an issue for the variation with parameters?

@michaelosthege
Copy link
Member Author

michaelosthege commented Jun 29, 2021

Should we open an issue for the variation with parameters?

The issue exists with #4494 and there was a PR #4692 that went stale because of architectural considerations 🙁

For the record: I considered the v.owner.op._print_name instead of v.owner.op.__class__.__name__.strip("RV") but for Normal and Uniform it's just N or U and that looked ugly.

@michaelosthege michaelosthege merged commit a449827 into pymc-devs:main Jun 29, 2021
@michaelosthege michaelosthege deleted the fix-4816 branch June 29, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor model_to_graphviz for v4
2 participants