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

WID-14 Improve the HTML representation on a Traited obj in Jupyter lab #639

Merged
merged 27 commits into from
Feb 3, 2023

Conversation

rarescodemart
Copy link
Contributor

@rarescodemart rarescodemart commented Jan 11, 2023

I converted the description for each model from RST format to HTML and I managed to render the mathematical equations (which are present in it) using MathJax.

@liadomide liadomide changed the title Wid 14 WID-14 Improve the HTML representation on a Traited obj in Jupyter lab Jan 11, 2023
@liadomide liadomide self-requested a review January 11, 2023 19:38
@liadomide liadomide added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 11, 2023
@liadomide
Copy link
Member

The result would look as in the attached image
Screenshot 2023-01-11 at 21 59 27

@liadomide liadomide requested a review from i-Zaak January 11, 2023 20:06
@i-Zaak
Copy link
Contributor

i-Zaak commented Jan 13, 2023

The result would look as in the attached image Screenshot 2023-01-11 at 21 59 27

This looks great! However is there a reason to duplicate the class docstrings in the dfun?

@liadomide
Copy link
Member

This looks great! However is there a reason to duplicate the class docstrings in the dfun?

dfun.__doc__ is expected to have only the equations (plus minimum text in some cases). The Class docstring is more generic and the eq there help for the overall description of the Model.

It is a decision we made a long time ago, to keep the eq in both places.
Some places which have less space to display (here, the PhasePlane in tvb web GUI) are better if only the equations are given. The overall Sphinx generated documentation can benefit for the class extended documentation.

It might be that we need to revise this decision...
For the current task we only took the existent convention/state and worked on the display in jupyterlab ;-)

@i-Zaak
Copy link
Contributor

i-Zaak commented Jan 13, 2023

I see, wasn't aware of that - in that case it of course makes sense. I guess the class docstring then would be shown for the component instance representation, to render e.g. the Coupling equations.

Alternatively one could keep the simulator representation as is (no equations) and show more details for the individual components separately.

@i-Zaak
Copy link
Contributor

i-Zaak commented Jan 13, 2023

And one more thing - do you want to do some more improvements for the HTML representations in this PR, or just focus on the equations?

@liadomide
Copy link
Member

liadomide commented Jan 13, 2023

And one more thing - do you want to do some more improvements for the HTML representations in this PR, or just focus on the equations?

Very good question. Could we delay answering until our next call 17 Jan? Then we can have a small demo with the current state, and we decide how to proceed...
Then, we would not merge until after 17th

@i-Zaak
Copy link
Contributor

i-Zaak commented Jan 13, 2023

@liadomide
Copy link
Member

Alternatively one could keep the simulator representation as is (no equations) and show more details for the individual components separately.

That might be a more generic strategy to follow (instead of the current code in this PR).
Because on Model we have dfun._doc__ (as used here), but Coupling has only an equation on the class doc (in this PR not shown)

@liadomide liadomide marked this pull request as draft January 13, 2023 12:15
tvb_library/tvb/basic/neotraits/info.py Outdated Show resolved Hide resolved
tvb_library/tvb/basic/neotraits/info.py Show resolved Hide resolved
tvb_library/tvb/basic/neotraits/info.py Outdated Show resolved Hide resolved
except ImportError:
def display_html(doc, raw):
return doc

Copy link
Member

@liadomide liadomide Jan 20, 2023

Choose a reason for hiding this comment

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

This has a nice outcome on the Class HTML representation, but I am not convinced the ipython dependency is healthy, being here at the very core (where the MetaClass for HasTraits is defined).

image

@maedoc @i-Zaak what do you think ?
Is it useful to keep also this Class HTML representation for jupyter, or users will be ok with only the instance representations (see comments bellow with exemples)

@liadomide
Copy link
Member

With only one dependency in tvb.basic.neotraits.info towards docutils, we get the following on instance level:

For a model instance :
image

A coupling instance:
image

And the simulator:
image

@i-Zaak would this outcome be ok ?

rarescodemart and others added 26 commits January 31, 2023 18:17
…ocstring in a TR-TD, otherwise it will fail to display correctly in case of nested table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants