Visualization with plotly

[effieHAN]

What problem do you want to solve?

Replace Bokeh with plotly for visualization.
The changes are made in the documentation file and visualization py file.

[ChristianZimpelmann]

Hi,

Thank you very much for that!

• I made a few comments and suggestions.
• Please add plotly and all other necessary packages (ipywidgets, ..?) to the environment file environment.yml. This is the reason that the tests are failing.
• Please add your contribution to changes.rst
• I get the error below when running the HowTo and no plot is shown. Do you have any idea? (Might be also an error on my end .. I haven't used plotly so far)
• I haven't had a closer look at the HowTo. Let's just leave it as is for now until we agreed on the function itself.

C:\.conda\envs\gettsim\lib\site-packages\jupyter_client\session.py:718: UserWarning:

Message serialization failed with:
Out of range float values are not JSON compliant
Supporting this message is deprecated in jupyter-client 7, please make sure your message is JSON-compliant

[LauraGergeleit]

Hi ! :)

I just took a look at the HowTo Guide and when running it through I always got the error below.
I googled the error and got this: "The Python "KeyError: 0" exception is caused when we try to access a 0 key in a a dictionary that doesn't contain the key. To solve the error, set the key in the dictionary before trying to access it or conditionally set it if it doesn't exist." (Maybe that helps)

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-20-7b4cbae92021> in <module>
----> 1 plot_dag(
      2     functions=policy_functions,
      3     selectors=[{"node": "zu_verst_kapitaleink_tu", "type": "neighbors"}],
      4     orientation ='v',
      5 );

~\projects\gettsim\gettsim\visualization.py in plot_dag(functions, source_code, targets, columns_overriding_functions, check_minimal_specification, selectors, orientation, showlabels, showlegend)
    424     # Even if we do not show the source codes , we need to remove the functions.
    425     dag = _replace_functions_with_source_code(dag)
--> 426     layout_df = _create_pydot_layout(dag, orientation)
    427     # prepare for the nodes dataframe including their url
    428     names = layout_df.index

~\projects\gettsim\gettsim\visualization.py in _create_pydot_layout(dag, orientation)
    154     # Remap layout from integers to labels.
    155     integer_to_labels = dict(zip(dag_w_integer_nodes.nodes, dag.nodes))
--> 156     layout = {
    157         integer_to_labels[i]: np.array(integer_layout[str(i)])
    158         for i in integer_to_labels

~\projects\gettsim\gettsim\visualization.py in <dictcomp>(.0)
    155     integer_to_labels = dict(zip(dag_w_integer_nodes.nodes, dag.nodes))
    156     layout = {
--> 157         integer_to_labels[i]: np.array(integer_layout[str(i)])
    158         for i in integer_to_labels
    159     }

KeyError: '0'

[sofyaakimova]

Hi we have made changes accordingly, except for the issue with if indentation andfig.add_scater `.

We consider it important to call fig.add_scater three times as first it draws edges and second the nodes. With respect to nodes it depends on the hover_info argument. When hover_info is false we need just name of the node, so it refers to the list of the function names. However, when we additionally want to display docstrings of the functions, we have to match the names and the docstrings. It becomes more complex as names in this case are just displayed next to the nodes while hover info includes the docstring. These two cases require 2 different fig.add_sacter functions. That’s how we realize different options.
If anything is still not clear or if you have any suggestions, please do let us know

@LauraGergeleit we are not completely sure what is the reason for the problem you face, as for us code works on both of computers. We can just give some suggestion that may help. We found that, in previous versions of environment this part of code ```
layout = {
157 integer_to_labels[i]: np.array(integer_layout[str(i)])

works only if it is displayed as follows```
 layout = {
    157         integer_to_labels[i]: np.array(integer_layout(i))

Probably the error you receive is also connected to this issue. Could you please try the new version and see if that works now or not?

[LauraGergeleit]

@LauraGergeleit we are not completely sure what is the reason for the problem you face, as for us code works on both of computers. We can just give some suggestion that may help.

Thanks, yes, maybe it is a problem on may end. However, I tried your suggestion and changed the code (see below) but still get the same error message when running the HowTo-File.

    integer_to_labels = dict(zip(dag_w_integer_nodes.nodes, dag.nodes))
    layout = {
        integer_to_labels[i]: np.array(integer_layout(i))
        for i in integer_to_labels
    }

[hmgaudecker]

I changed it back to the original version without the string-conversion. @sofyaakimova @effieHAN, do you remember what made you change that? Runs without it in a freshly created environment -- if that is different for any one of you, we'll need to investigate further.

@LauraGergeleit, my hunch is that the changed code did not work for you because of a missing kernel restart. Could you test again, please?

[effieHAN]

As far as we remembered, we changed the code to string-conversion because in the new environment we got a KeyError (see attached screenshot), and Christian suggested we convert it to string. For us, it wouldn't run if we use the original version.

[hmgaudecker]

Thanks! Will be hard to track down what exactly the difference relative to the current version is, let's continue watching it whether any error like that pops up again.

[codecov]

Codecov Report

Merging #403 (a4da5c2) into main (e6d0332) will increase coverage by 0.29%.
The diff coverage is 97.33%.

@@            Coverage Diff             @@
##             main     #403      +/-   ##
==========================================
+ Coverage   92.13%   92.42%   +0.29%     
==========================================
  Files          76       76              
  Lines        3597     3606       +9     
==========================================
+ Hits         3314     3333      +19     
+ Misses        283      273      -10     

Impacted Files	Coverage Δ
docs/conf.py	100.00% <ø> (ø)
gettsim/transfers/grundrente.py	92.72% <ø> (ø)
gettsim/visualization.py	97.28% <96.82%> (+4.42%)	⬆️
gettsim/tests/test_visualizations.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[sofyaakimova]

I just tried the latest version and on my computer it works now without string conversion. No idea what caused the problem before.

[LauraGergeleit]

I changed it back to the original version without the string-conversion. @sofyaakimova @effieHAN, do you remember what made you change that? Runs without it in a freshly created environment -- if that is different for any one of you, we'll need to investigate further.

@LauraGergeleit, my hunch is that the changed code did not work for you because of a missing kernel restart. Could you test again, please?

I guess you were right, now it runs through!

[ChristianZimpelmann]

Thank you for the work so far!

I made a couple of comments.

[ChristianZimpelmann]

Good job, thanks! I pushed a commit with small changes to tutorial and the docstring and made very few new comments. I have one bigger open question

• in case I select hover_source_code=True what happens to the node name/label in case those are not shown for all nodes ((len(names) > 10) or (show_labels is False)). If I see correctly, we don't show the name of the node at all in this case? Can we improve on the current behavior? But it is not super important as it is only an experimental feature.

Otherwise, we will be able to merge very soon!

[LauraGergeleit]

Hi!
I looked through the HowTo-Guide again, now that it runs through, and checked for any mistakes left. Here are some small things I came across:

Introduction:

• This function**s** has two arguments and one of them passes parameters to the function. Get rid of the s
• Each node is either a function**s** or a variable. Get rid of the s
• By clicking on a node, you are redirected to the documentation of **gettsim** and if the variable is computed by a function, the function's documentation is displayed. Write GETTSIM in capital letters
• Functions inside GETTSIM are a little bit special. Take for example abgelt_st_tu which is documented here. The signature of the function is This sentence is double and can be deleted at the end of the paragraph
• Here is the complete signature**.** My suggestion would be to use a colon instead

Selectors:

• 3. "select" specifies whether the nodes should be selected or de-selected. If you do not specify "select" it is assumed to be True.**Let us go through the keys of the dictionary one by one.** This part is double and can be deleted

De-selecting Nodes:

• For a simple and silly example, we want to reproduce the graph with the single node for kapitaleink_brutto_tu **after**, but starting from the last plot which also showed zu_verst_kapitaleink_tu. 'after' makes no sense here, I think it can just be deleted
• The second key in the de-selects "zu_verst_kapitaleink_tu". Something is missing in that sentence, I would suggest: The second selector or dictionary in the list de-selects "zu_verst_kapitaleink_tu".

Ancestors and Descendants:

• To select "zu_verst_kapitaleink_tu" which is the calculated taxable capital income per tax unit and all its ancestors, do the following**.** My suggestion would be to use a colon instead

Orientation:

• By default orientation of graph is vertical. This sentence seems a bit incomplete. I would suggest: By default, the orientation of the graph is vertical.

Labels:

• By default when number of nodes is at most 10 all names are displayed next to the node. I would suggest the following to make this sentence easier to read: By default, when the number of nodes is at most 10, all names are displayed next to the respective node.

Hover info (source code):

• It is also possible to address the source code of the function. By setting plot_dag(..., hover_source_code=True) I would combine these two sentences to one: It is also possible to address the source code of the function when hovering over the node by setting plot_dag(..., hover_source_code=True)

I hope that helps. In case you have questions, left me know and please note that some of the points are just my suggestions.

[ChristianZimpelmann]

I just had one more idea. Could we please change the possible values of the argument show_labels as follows:

• True -> show labels (independent of number of nodes)
• False -> don't show labels (independent of number of nodes)
• None (default) -> show labels if number of nodes <= 10

If you think that makes sense, please change it in code, docstring, and tutorial

[sofyaakimova]

Hi Christian, we made the changes you and Laura made correspondingly. Could you please have a look?
There is also one thing that grabbed my attention. In the tutorial, we have: "By clicking on a node, you are redirected to the documentation of gettsim and if the variable is computed by a function, the function's documentation is displayed" in the introductory part. However, we haven't still implemented this feature, as the click function that allows us to redirect to external links does not work inside our function and we couldn't find the solution.
So, should we for now delete this sentence from our tutorial?

[ChristianZimpelmann]

Dear Sofya,

Thank you very much! Sorry for the late response. I was very busy this week, but will be faster from now on.

So, should we for now delete this sentence from our tutorial?

That sounds great!

I have two requests:

• Can you resolve all comments above that you have addressed?
• Can you run precommit hooks over the code.. one tests seems to be failing because of a trailing whitespace. Let me know if you have questions about it.

Best, Christian

[ChristianZimpelmann]

With sphinx 5.1.1. which came out 2 weeks ago, readthedocs does not compile anymore due to an error of automodapi. See my comment in this issue: astropy/sphinx-automodapi#148

For now, I fixed sphinx at 5.1.0 (and python at 3.8 in .readtehdocs.yaml which readthedocs seems to require).

[ChristianZimpelmann]

All done now and we can merge.

Thanks for the great work!