Use scooby for Versions #792

prisae · 2019-07-01T07:36:29Z

Use scooby internally for Versions. Now that scooby exists (thanks @banesullivan ) we should use it to simplify our lives. This removes lots of code from SimPEG, and will make maintenance easier down the road.

It is a bit a shame this comes only a few days after https://github.com/simpeg/simpeg/releases/tag/0.11.5 - but the development of scooby came a bit unexpected and fast.

However, there is absolutely NO change to the API / for the user. Everything remains the same.

In the end, the work we did with Versions in SimPEG (and in empymod/emg3d) all led to scooby, so it was well invested time ;-)

prisae · 2019-07-01T07:37:04Z

Btw, what do you think about having the same in discretize too? Now it would just mean a few lines of code...

banesullivan · 2019-07-01T07:38:08Z

Or what if it were only in discretize (and SimPEG called discretize's function for backwards compatibility)?

prisae · 2019-07-01T07:39:38Z

I changed

**Parameters**

to

Parameters
----------

I assume SimPEG is moving to numpydoc-style too, right?

prisae · 2019-07-01T07:42:24Z

Or what if it were only in discretize (and SimPEG called discretize's function for backwards compatibility)?

No, I don't think that is good. SimPEG is the main package. So it should have it. It is where everything comes together. discretize could have it too, as it can be used standalone, but not necessary. Because always when you use discretize then you will use something else too; e.g. SimPEG, or emg3d, or similar. And those tools should have it, IMO.

banesullivan · 2019-07-01T07:46:20Z

That makes sense so that SimPEG could specify specific packages and hardware which might not be relevant to other packages leveraging discretize.

codecov · 2019-07-01T08:04:47Z

Codecov Report

Merging #792 into simulation will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           simulation     #792   +/-   ##
===========================================
  Coverage       71.22%   71.22%           
===========================================
  Files             118      118           
  Lines           19092    19092           
===========================================
  Hits            13599    13599           
  Misses           5493     5493

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 743f380...0f83e38. Read the comment docs.

prisae · 2019-07-01T17:08:38Z

We might think of renaming it from Versions to Report. This would be backwards incompatible (again), so it would have to be before #786 .

The reason is that @banesullivan and I think/hope/expect that scooby will become more widely adapted, and in scooby it is called Report (in the end, it does a bit more than only showing the versions). So it might become common that modules have a module.Report()-instance, and then it might be useful if SimPEG follows that.

Just thinking out loud, ideas/opinions welcome.

lheagy

Thanks @prisae and @banesullivan for making this happen 🎉

Just one comment wrt making it a soft dependency rather than putting it in the setup.py. Other than that, this looks great!

Also 👍 from me on renaming to be consistent with scooby going forwards. It is a straightforward change in people's code, so might as well take care of it right away

lheagy · 2019-07-02T15:30:04Z

setup.py

@@ -43,6 +43,7 @@
        'vectormath>=0.2.0',
        'discretize>=0.4.0',
        'geoana>=0.0.4'
+        'scooby>=0.3.0',


What do you think of making this a soft dependency? Since it isn't strictly necessary for running simulations or inversions with SimPEG, I would be inclined to make it a soft rather than a hard dependency

prisae · 2019-07-02T15:54:11Z

Yes, I was thinking about making it a soft dependency just like matplotlib in discretize. I opted not to, as scooby is tiny compared to matplotlib.

I'll implement that! If not today than later in the week.

prisae · 2019-07-02T16:19:37Z

A very quick implementation, not really tested yet...

banesullivan · 2019-07-02T16:24:30Z

My input, FWIW: while yes, we are still house-training scooby as it's just a puppy of a project and you might not want users to be forced to install something like that, I went ahead and made scooby a hard dependency of PyVista and plan to make it so with other projects I work on.

Why?

Lately, PyVista has been gaining a lot of users that don't have a strong Python background and they've been happy to report bugs/generate issues but it was becoming an challenge to work with numerous users to report everything we needed to know about the environment and the computer. My solution: have scooby as a hard dependency so that as soon as a user reports a bug, we have them simply run pyvista.Report() and it'll just work (as having non-Python-aficionados install a new library was too cumbersome at times and they might not follow up on the issue).

So I guess scooby started as a reproducibility tool and has morphed into more of a bug-reporting tool for how I use it at least.

Edit: a minority of our users leverage anaconda, so that's really where the issue arose

prisae · 2019-07-02T18:29:50Z

@banesullivan thanks for the input. @lheagy I can leave the soft dependency in or make it hard again, depending on what you folks want. Maybe you can raise it today at the meeting?

Either way: The requires-function could be useful for other soft dependencies too, so it should not live in printinfo. I moved it into codeutils and renamed it to requires_module, as a requires exists already in the same file for variables.

Also, printinfo (the old Version), was quite large, so a separate file was justified. Now, with scooby, it is a tiny thing. So I moved it into codeutils too, removing the file printinfo.py altogether.

prisae · 2019-07-02T18:48:05Z

There is actually a much simpler way in this case than requires_module:

# scooby is a soft dependency for SimPEG
try:
    from scooby import Report as ScoobyReport
except ImportError:
    class ScoobyReport:
        def __init__(self, additional, core, optional, ncol, text_width, sort):
            print('\n  *ERROR*: `SimPEG.Report` requires `scooby`.'
                  '\n           Install it via `pip install scooby`.\n')

This is all that is needed to make it a soft dependency. And it prints the useful info.

Edit: I edited the implementation to avoid a name-clash with the actual Report class later.

banesullivan · 2019-07-02T19:38:33Z

@prisae - I like that soft dependency implementation a lot! Maybe we should tag that for scooby's docs for whenever we get around to making those

prisae · 2019-07-02T19:41:14Z

Yes, like this it is very minimal. We could even put it on the README of scooby, what you think? Or later in the docs...

banesullivan · 2019-07-02T19:41:54Z

Maybe just the docs - maybe make an issue over there so we don't forget about this?

prisae · 2019-07-02T19:47:28Z

Done: banesullivan/scooby#32

prisae · 2019-07-03T07:03:39Z

I believe the fail is ones again related to the Google Cloud SDK thingy...

lheagy · 2019-07-14T20:38:35Z

Thanks for pushing on this! I am at a conference this week and am again behind on things... but I will jump back in in a week or two

prisae · 2019-07-24T07:11:21Z

Two warnings from the Travis-log (run 10 - docs) which might need some action?

WARNING: intersphinx inventory 'http://discretize.simpeg.xyz/en/latest/objects.inv' not fetchable due to <class 'requests.exceptions.HTTPError'>: 404 Client Error: Not Found for url: http://discretize.simpeg.xyz/en/latest/objects.inv
generating gallery...
WARNING: Duplicate file name(s) found. Having duplicate file names will break some links. List of files: ['../examples/04-grav/plot_inversion_linear.py']

prisae · 2020-02-07T11:53:40Z

@jcapriot , could this PR (or the branch scooby) be merged into the simulation branch?

jcapriot · 2020-02-07T22:02:59Z

I'm going to switch the merge request to the simulation branch, and merge those changes into here to check, but I don't think this will cause any issues.

A few things, cython is not technically a requirement for SimPEG, it is a requirement of discretize. The old Versions was the only place cython was ever actually used in SimPEG. I'll be removing it from SimPEG's setup.py shortly.

prisae · 2020-02-08T05:30:28Z

Great, thanks @jcapriot !

# Conflicts: # SimPEG/__init__.py # SimPEG/utils/printinfo.py # docs/content/api_core/api_Utils.rst # examples/06-dc/plot_dc_schlumberger_array.py # examples/06-dc/read_plot_DC_data_with_IO_class.py # examples/07-fdem/plot_loop_loop_2Dinversion.py # examples/09-tdem/plot_inductive_src_permeable_target.py # requirements_dev.txt

Use scooby for Versions

134c4fe

prisae requested a review from lheagy July 1, 2019 07:37

Provide all args

502ba27

Explicit args pasing

5f6667a

lheagy reviewed Jul 2, 2019

View reviewed changes

Make scooby a soft dep; Versions->Report

748b280

prisae added 2 commits July 2, 2019 20:25

Move to codeutils

a18d8ed

Typo

da25600

Simpler way for requires_module

a70cd1d

prisae added 2 commits July 2, 2019 20:48

typo

a4a2201

Avoid name clash

b882a5e

Merge branch 'master' into scooby

c435993

prisae added 2 commits February 7, 2020 13:00

Add conda-forge possibility

071f3a5

Fix wrong bracket

743f380

banesullivan approved these changes Feb 7, 2020

View reviewed changes

jcapriot changed the base branch from master to simulation February 7, 2020 22:06

jcapriot merged commit 61471da into simulation Feb 20, 2020

jcapriot deleted the scooby branch February 20, 2020 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use scooby for Versions #792

Use scooby for Versions #792

prisae commented Jul 1, 2019

prisae commented Jul 1, 2019

banesullivan commented Jul 1, 2019 •

edited

prisae commented Jul 1, 2019 •

edited

prisae commented Jul 1, 2019

banesullivan commented Jul 1, 2019 •

edited

codecov bot commented Jul 1, 2019 •

edited

prisae commented Jul 1, 2019

lheagy left a comment •

edited

lheagy Jul 2, 2019 •

edited

prisae commented Jul 2, 2019

prisae commented Jul 2, 2019

banesullivan commented Jul 2, 2019 •

edited

prisae commented Jul 2, 2019

prisae commented Jul 2, 2019 •

edited

banesullivan commented Jul 2, 2019

prisae commented Jul 2, 2019

banesullivan commented Jul 2, 2019

prisae commented Jul 2, 2019

prisae commented Jul 3, 2019

lheagy commented Jul 14, 2019

prisae commented Jul 24, 2019

prisae commented Feb 7, 2020

jcapriot commented Feb 7, 2020

prisae commented Feb 8, 2020

Use scooby for Versions #792

Use scooby for Versions #792

Conversation

prisae commented Jul 1, 2019

prisae commented Jul 1, 2019

banesullivan commented Jul 1, 2019 • edited

prisae commented Jul 1, 2019 • edited

prisae commented Jul 1, 2019

banesullivan commented Jul 1, 2019 • edited

codecov bot commented Jul 1, 2019 • edited

Codecov Report

prisae commented Jul 1, 2019

lheagy left a comment • edited

Choose a reason for hiding this comment

lheagy Jul 2, 2019 • edited

Choose a reason for hiding this comment

prisae commented Jul 2, 2019

prisae commented Jul 2, 2019

banesullivan commented Jul 2, 2019 • edited

prisae commented Jul 2, 2019

prisae commented Jul 2, 2019 • edited

banesullivan commented Jul 2, 2019

prisae commented Jul 2, 2019

banesullivan commented Jul 2, 2019

prisae commented Jul 2, 2019

prisae commented Jul 3, 2019

lheagy commented Jul 14, 2019

prisae commented Jul 24, 2019

prisae commented Feb 7, 2020

jcapriot commented Feb 7, 2020

prisae commented Feb 8, 2020

banesullivan commented Jul 1, 2019 •

edited

prisae commented Jul 1, 2019 •

edited

banesullivan commented Jul 1, 2019 •

edited

codecov bot commented Jul 1, 2019 •

edited

lheagy left a comment •

edited

lheagy Jul 2, 2019 •

edited

banesullivan commented Jul 2, 2019 •

edited

prisae commented Jul 2, 2019 •

edited