SasView / sasmodels

Package for calculation of small angle scattering models using OpenCL.
BSD 3-Clause "New" or "Revised" License
15 stars 27 forks source link

More informative error message when sasview is not on the python path #510

Closed pkienzle closed 2 years ago

pkienzle commented 2 years ago

The import error now says that sasview/src is missing from the python path rather than module sas not found. This is still somewhat cryptic. The full instructions should say that sasview can be downloaded with "git clone https://github.com/sasview/sasview.git" but that is a bit much for an exception text string.

See #467.

wpotrzebowski commented 2 years ago

@lucas-wilkins will come up with better suggestion for handling error

pkienzle commented 2 years ago

It is not worth any extraordinary efforts. Once the data readers are available as a separate package the issue can be addressed properly.

lucas-wilkins commented 2 years ago

No extraordinary change made

lucas-wilkins commented 2 years ago

No extraordinary change made

or at least there's not now ;)

pkienzle commented 2 years ago

Some preferences:

  1. Regarding f-strings, sasmodels still supports python 2.7. If we start using python 3 features then semantic versioning would require that we release it as 2.0. I've been avoiding this by not using python 3 features.

  2. I prefer the "Data loader" not available rather than "sas" not available (which is what ie.name expands to when sasview is not on the path). It feels more user focused. Also, the particular module is available further up on the traceback.

  3. I prefer the same variable name to hold the exception. sasmodels uses exc elsewhere, so use that rather than ie.

Or perhaps change all of them to e since that is the most common in the standard library:

 312 e
 200 exc
  94 err
  90 msg
  17 ex
  15 why
  15 v
  12 pe
   9 error

and in my site-packages directory:

1715 e
 472 err
 394 exc
  77 error
  68 msg
  41 caught_exc
  34 ex
  26 v
  19 exception

SasView uses a mixture of ex in the qt gui, mostly exc in sascalc, and a variety of forms in the data loaders, with e as the most common.

pkienzle commented 2 years ago

It looks like I've decided that the matrix multiplication operator @ is compelling enough in another thread. I guess that means f-strings are okay. I'm still not convinced I want to bump to version 2 because of this patch. Maybe 1.1?

lucas-wilkins commented 2 years ago

e, pe, and v are probably equivalent in the sense that they are the lower case initials of the exception class. I would usually use e, or maybe ex. It's just a very unusual thing to catch.

Personally, I would think the overlap between users that know what "Data loader" means, and those that wouldn't prefer a more specific error message, would be small.

Roll it back if you like.

pkienzle commented 2 years ago

These are minor preferences. I'll leave it to someone else to decide if they want to rollback or merge as is. Regardless, we should revisit this when the data loaders are available as a separate package.

lucas-wilkins commented 2 years ago

My commits were just a suggestion, I'm happy either way too.