Skip to content

Update LKJ notebook to v4 #287

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 2 commits into from
Mar 8, 2022
Merged

Update LKJ notebook to v4 #287

merged 2 commits into from
Mar 8, 2022

Conversation

fonnesbeck
Copy link
Member

Cleanup and update to v4 and Aesara

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ghost
Copy link

ghost commented Mar 8, 2022

Not sure what the jupytext failure entails. There is just a lot of embedded image garbage, which makes it hard to diagnose.

Copy link
Collaborator

@AlexAndorra AlexAndorra left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @fonnesbeck

@fonnesbeck fonnesbeck merged commit 927be3d into pymc-devs:main Mar 8, 2022
@fonnesbeck fonnesbeck deleted the lkj_v4 branch March 8, 2022 17:48
@OriolAbril OriolAbril mentioned this pull request Mar 9, 2022
3 tasks
@OriolAbril
Copy link
Member

Sent a PR to fix CI.

Not sure what the jupytext failure entails.

Notebooks have now each 2 files, a ipynb one and a myst one. The jupytext pre-commit keeps them synced, you only need to care and work on the ipynb one when writing/executing notebooks. When reviewing however, the ReviewNB has too many blind spots, and the myst view is very useful; it shows the text changes much more clearly. Reviewnb messes up big time when the text is not changed but it's formatting is and starts showing html tags as if they were part of the diff, it ignores the breaklines (as it should when rendering markdown, only while lines are relevant, but with myst when using directives breaklines are important) and also ignores completely all the tags and metadata of the notebooks which can be used to hide code cells or outputs that are interesting but not key to the notebook, to add captions and/or alt text to code generated plots... I shared the link to #283 which added this syncing and we also discussed that in a documentation meeting where we decided the extra pre-commit was worth it to make sure we can review easily even when using all these features I mentioned.

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.

3 participants