Improve user interface

[kallewesterling]

Fixes #159.

[codecov-commenter]

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 ⚠️

Additional details and impacted files

@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (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

[mastoffel]

Great job @kallewesterling, this is exactly what I was thinking about.

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.

[kallewesterling]

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

[mastoffel]

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]

Fixed the high-level (1) above in 6dab835

[kallewesterling]

Fixed (2) in daec70f

[kallewesterling]

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

[mastoffel]

Very nice @kallewesterling , notebook printing looks great, and printing.py is a good place for this.

Just one more comment below.

[mastoffel]

works well for me

[mastoffel]

one after thought here: Maybe this would be most useful if the argument names (rather than attribute names) would be in brackets. Would you agree?

If so, the two things to change are:

"# training set samples (train_idxs)",
"# test set samples (test_idxs)",

to
"test set size (test_set_size)"
(and the value could be 20% or 0.2)

and
"Cross-validation strategy (cv)",
to
"Cross-validation strategy (fold_strategy)",

[kallewesterling]

Ah, that makes sense. I was thinking that these should refer to any properties that exist on the object (hence, cv instead of fold_strategy as self.cv has been set; same with train_idxs and test_idxs as those are also existing as properties, whereas test_set_size is not a property on the object)... But since we're just showing the setup settings, I suppose it makes sense!

[kallewesterling]

See: c0504de

[mastoffel]

ok, thanks @kallewesterling ! Feel free to merge!