Skip to content

Conversation

cscheffler
Copy link
Contributor

The model graph (graphviz) output currently strips the leading "V" from a "VonMises" random variable node. The reason is that the current code uses .strip("RV") to remove characters which is too aggressive. It has been replaced with a check to see if the node name ends with "RV" and to remove those two characters if it does.

There is also a minor typo where style = None was written as syle = None (the t is missing) but this has no impact since style is set to None earlier in the code anyway.

Bugfixes

  • Fixed model graph node names to remove RV from end only and not the start.

@cscheffler cscheffler marked this pull request as ready for review July 6, 2022 11:01
Copy link
Member

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

The changes look reasonable and once the CI pipeline goes green we can merge it 👍

@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #5953 (84ad3ec) into main (b2393d5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5953   +/-   ##
=======================================
  Coverage   89.54%   89.54%           
=======================================
  Files          73       73           
  Lines       13271    13273    +2     
=======================================
+ Hits        11883    11885    +2     
  Misses       1388     1388           
Impacted Files Coverage Δ
pymc/__init__.py 100.00% <100.00%> (ø)
pymc/model.py 88.20% <100.00%> (ø)
pymc/model_graph.py 93.70% <100.00%> (+0.10%) ⬆️

@michaelosthege michaelosthege merged commit dfd018e into pymc-devs:main Jul 6, 2022
@cscheffler cscheffler deleted the fix-graph-node-name branch July 6, 2022 19:40
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