Plugin framework is broken

[butler]

several people have mentioned problems with plugins recently. Thorough testing, particularly by @tcbennun and @gonzalezm, has revealed that the problem seems to lie in the loading of the plugin and the math package (quoting from @gonzalezm):

1. I create a simple model without math functions (A+B*x): OK

2. Using a math function as in the tiptool (A+B*cos(2*pi*x)): The editor gives a success message and the python file is correctly generated and saved in plugin_models. But when trying to load it in SasView, an error appears:

2018-07-20 12:11:54 : Traceback (most recent call last):
File "sas\sascalc\data_util\calcthread.pyc", line 274, in _run
TypeError: unsupported operand type(s) for *: 'NoneType' and 'float'

3. I edited the generated file to check it and everything looks correct. Then it is enough to modify it (e.g. add a space) or save it with a different name, and then the model can be loaded and it will work.

4. As the model is correct, closing and relaunching SasView will also make the model available and it works. This could be a roundabout, i.e. first create the model, then close SasView and then restart. But it is clearly not satisfactory.

Investigating further it seems the root cause may be the loading for testing of new plugins (quoting now from @tcbennun):

It seems that when you create a new plugin model, the new Python file is loaded as a module from source twice when it is checked, in pyconsole.py, check_model(). The first time, it is loaded with sasview_model.py, load_custom_model() (just to check it can be run), and the second time, with generate.py, load_kernel_module() (this happens during the unit tests).

If load_custom_model() had been used twice there would be no problem, because that function caches previously-loaded modules and prevents them from being reloaded. But load_kernel_module() is separate, so the module is loaded again.

The GUI generally only uses load_custom_model(), as far as I can tell, so this bug might not appear elsewhere at all.

Theoretically this problem shouldn’t exist (Python docs claim that reloading a module from source re-initializes the module safely), but we’re seeing that the module’s namespace is not populated correctly for whatever reason the second time round, so it’s missing those maths functions which are imported at the top of the file.

Further testing by myself shows that actually any plugin, if tested (either by the new editor module because it is just created or by running run → Check model in the advanced model editor) once, may pass the test (it will not the second time) but will now throw errors when used. This includes downloading a model from the marketplace. Just running it as is works, as soon as it is loaded into the editor and checked (regardless of whether changes have been made) it no longer works. Exiting SasView and restarting clears the problem.

This issue is probably identical to the one that prevented #885 from being implemented (in fact we rolled back polydispersity completely). Once this gets fixed we should retest

[butler]

Further sleuthing with the help of Paul Kienzle using the built in python console confirms Torin's findings: there are two separate loaders involved which for some reason both want to keep a separate copy of the model. It appears that when the sasmodels unit tests load the model the second time the math globals are overwritten so that rather than functions they become "None" objects. Unfortunately, as I understand it, SasView uses the first version (where all the math functions are now "NONE" type objects) leading to the errors being thrown.

Paul Kienzle points out this is probably an old problem going back to the Grenoble code camp where it was found that numpy needed to be imported within the I(q) function itself to work? He also thinks he can force sasmodels to use the version loaded by SasView so that it is not loaded twice which should fix the issue.

The longer term question however, that maybe should become a shorter term one, is that sasmodels.sasview was meant as a temporary shim so that we could release SasView updates while in the process of converting to the new sasmodels package properly. With version 4.2.0, that conversion is essentially complete and thus the time has come that for that shim to disappear and for SasView to call sasmodels directly rather than through .sasview. But that is a fair amount of work and 4.2.0 will clearly still be using the shim. Version 5 however should definitely be working to sasmodels?

[Paul Kienzle <pkienzle@…>]

In 2a6058cb3b646db9e7b57a6ce2ff246d32fa5518/sasview:

fix broken plugin model tests. Fixes #1142

[Paul Kienzle <pkienzle@…>]

In fa374b0f1a207d4b01b7cb3dc0bfc3cef6bc64fa/sasview:

Save before pushing… Fixes #1142

[butler]

The fix added in SasView PR 161 seems to fix part of the problem? maybe? but not really? Attached are two files, one from Maria Valldeperas in Lund clearly using a slightly older version of the plugin editor, but trivially so. The other created by copying and pasting all the bits from Maria's version using the new model editor using the PR build (so the 4.2.0-beta version of the editor). Maria's throws the usual error of the math functions being of type "NoteType" while the new one (which looks very similar) does not. Note that for Maria's model there is no need to load it in the editor and run the test. Just dropping it in the plugin folder and launching SasView then try choosing that plugin from the fitpanel. Moreover, by removing the math functions from Maria's original file (cos and exp) and leaving simple arithmetic, the model loads and runs fine - further suggesting it is the same problem with the math functions.

a couple of questions:

• Do others see the same thing?
• If so, does this suggest the problem still lurks and depending on the user could pop up again even with the "new" editor?
• If not, should we abandon all hope of backward compatibility? I find this a bit troubling given the almost identical nature of these files.

[butler]

Maria's original version

[butler]

New version of Maria's model

[Paul Kienzle <pkienzle@…>]

In e2663b7a7ed44d36d60ec897fa92d1309bcf6d8b/sasview:

remove cached model before attempting reload. Refs #1142

[pkienzle]

L3_Porcar_et_al.py has a mixture of line endings:

\r\n
\r
\r\r\n

Turns out that this is a red herring, and the problem was elsewhere, but we may want to figure out whether it is our editor which is doing this.

[GitHub <noreply@…>]

In 3bfcd9ea3b637986d859554ea178d94819486a2f/sasview:

Merge pull request #161 from SasView?/ticket-1142-plugin-reload

Ticket 1142 plugin reload

addresses #1142 hopefully for the last time - actually will leave the ticket open as there is an improved fix that is waiting in a sasmodels PR to be merged after the release - move ticket to 4.3 now

[butler]

Actually, in hindsight this will be too confusing so have opened another ticket for the refactoring and will now close this one.

[GitHub <noreply@…>]

In c6084f192171c34cf1111bfae238f49f5ebf15f5/sasmodels:

Merge pull request #75 from SasView?/ticket-1142-plugin-reload

refactor tests so that gui doesn't reload module each time. Refs #1142.

[butler]

Some fixes for 4.2.1 added after closing moving to 4.2.1