alan-turing-institute / autoemulate

emulate simulations easily
MIT License
15 stars 1 forks source link

Improve user interface #195

Closed kallewesterling closed 4 months ago

kallewesterling commented 4 months ago

Fixes #159.

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 82.85714% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 88.00%. Comparing base (69932f1) to head (daec70f).

Files Patch % Lines
autoemulate/printing.py 78.57% 6 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #195 +/- ## ========================================== - Coverage 88.09% 88.00% -0.09% ========================================== Files 44 44 Lines 2083 2118 +35 ========================================== + Hits 1835 1864 +29 - Misses 248 254 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 4 months ago

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  autoemulate
  compare.py
  printing.py 9, 14, 61, 115-117
Project Total  

This report was generated by python-coverage-comment-action

kallewesterling commented 4 months ago

Two high level things:

  1. Could we put this into a separate module to keep compare clean?
  2. What would be great eventually (though no need for this now if you're tight on time) is to check whether autoemulate is run in a notebook and if so display the table with display(HTML(df.to_html())) from from IPython.display, which looks a bit nicer.
  1. Do you mean to move it to, say, utils.py? I was thinking, as it pertains to the class specifically, it makes sense to have it as a method of the compare.AutoEmulate class?
  2. This sounds like a great idea. I haven't found a super way of checking whether the script is run in a notebook or not..

    I have seen two different versions of trying to resolve it (see this source for instance) -- any idea which ones is preferrable?

    1.

    try:
       __IPYTHON__
       _in_ipython_session = True
    except NameError:
       _in_ipython_session = False

    2.

    try:
       from IPython import get_ipython
       ip = get_ipython()
       if ip is None:
               # we have IPython installed but not running from IPython
               return False
           else:
               from IPython.core.interactiveshell import InteractiveShell
               format = InteractiveShell.instance().display_formatter.format
               if len(format(_checkhtml, include="text/html")[0]):
                   # TODO: need to check for qtconsole here!
                   return True
               else:
                   return False
       except:
           # We do not even have IPython installed
           return False
mastoffel commented 4 months ago

Two high level things:

  1. Could we put this into a separate module to keep compare clean?
  2. What would be great eventually (though no need for this now if you're tight on time) is to check whether autoemulate is run in a notebook and if so display the table with display(HTML(df.to_html())) from from IPython.display, which looks a bit nicer.
  1. Do you mean to move it to, say, utils.py? I was thinking, as it pertains to the class specifically, it makes sense to have it as a method of the compare.AutoEmulate class?
  2. This sounds like a great idea. I haven't found a super way of checking whether the script is run in a notebook or not.. I have seen two different versions of trying to resolve it (see this source for instance) -- any idea which ones is preferrable?

    try:
       __IPYTHON__
       _in_ipython_session = True
    except NameError:
       _in_ipython_session = False
    try:
       from IPython import get_ipython
       ip = get_ipython()
       if ip is None:
               # we have IPython installed but not running from IPython
               return False
           else:
               from IPython.core.interactiveshell import InteractiveShell
               format = InteractiveShell.instance().display_formatter.format
               if len(format(_checkhtml, include="text/html")[0]):
                   # TODO: need to check for qtconsole here!
                   return True
               else:
                   return False
       except:
           # We do not even have IPython installed
           return False

Ah sorry, I wasn't very clear. I think it's good to have it as a autoemulate.compare method, but I'd still put the majority of code in a separate module (say pretty_printing.py or so), and then import _print_setup for the autoemulate.compare print_setup method. This is just so that autoemulate.compare doesn't become too big with all it's methods.

Good question with the checking for whether it's run in a notebook. I couldn't find a super clear answer either. I'll have a closer look tomorrow, but both versions seem to work.

kallewesterling commented 4 months ago

Fixed the high-level (1) above in https://github.com/alan-turing-institute/autoemulate/pull/195/commits/6dab835c6bf949bfbe151adc305cdb10271be535

kallewesterling commented 4 months ago

Fixed (2) in https://github.com/alan-turing-institute/autoemulate/pull/195/commits/daec70f5b742868c240c24424f3860c575101188

kallewesterling commented 4 months ago

OK @mastoffel I think that's all the stuff addressed! Check out the branch now :)