Maintain old widget for 0D and PseudoCounters in TaurusValue

[reszelaz]

sardana-org/sardana#1203 introduces new widget for experimental channels which allows acquisition. Since 0D and PseudoCounters do not allow experimental channel acquisition still use the old widget for them.

[reszelaz]

Hi Carlos,
Could you please check what is wrong with the testsuite, especially for py3-qt5? It seems like there are some core tests failing and I think that this PR changes should not affect these tests.
Thanks a lot!

[cpascual]

• The py27qt4 tests are erroring almost always in travis lately for unknown reasons, (the testsuite finishes without errors but it crashes after it). It does not happen when I run the tests locally. I would ignore it after checking in the logs that the testsuite indeed finished without errors

• the rest of the targets are now green (I just re-triggered the py35qt5 target which was red). They are not robust (see CI showing random segfaults #1042). It is a PITA and we really should do something about it (I've some hopes in that the fix of DS crashes when unsubscribing from events in __del__ tango-controls/pytango#292 may help here)

[reszelaz]

Great! Thanks for checking that!
Then do you think we could merge it (merging at the same time sardana-org/sardana#1203)?
Is there any other place where the TV map is declared or the custom settings module is the only one?

[cpascual]

I am not totally convinced about these changes. I feel uncomfortable with the fact that this would break installations in which taurus is updated while sardana is not (it is not a terrible break, but the 0D and pseudocouunters would start being shown as generic devices). This is more important for people with py2 systems who won't update to sardana 3.x straight away.

IMHO, a better solution is to add some transitional logic in the new PoolChannelTV to support the 0D and PseudoCounters as part of sardana-org/sardana#1203

...or alternatively, use monkey-patching from sardana to update tauruscustomsettings.T_FORM_CUSTOM_WIDGET_MAP until the new widget adds proper support

[cpascual]

IMHO, a better solution is to add some transitional logic in the new PoolChannelTV to support the 0D and PseudoCounters as part of sardana-org/sardana#1203

Let me elaborate: I am not saying that the complete support of 0D and PseudoCounters should be added now as part of sardana-org/sardana#1203 . What I mean is that some check is temporarily implemented in the setModel of the new PoolChannelTVthat disables the new features if the model corresponds to a 0D or PseudoCounter.

[cpascual]

Is there any other place where the TV map is declared or the custom settings module is the only one?

I've checked and I'd say that tauruscustomsettings is the only place (apart from some irrelevant "demo" code).

Still, I think it is worth considering making this setting an entry-point to get rid of this sardana- and tango- centric bit in taurus. This is already identified in APPENDIX IV of TEP13.

If we implemented this entry-point, then it would be straight away for sardana-org/sardana#1203 to register whatever map it needs for its own widgets (without worrying about side-efects in taurus).

So, I see 3 alternatives for achieving the objectives of this PR without breaking compatibility:

1. temporarilly adding a tango class check in the new PoolChannelTV.setModel() to handle the same classes as the old one
2. monkey-patching from sardana to update tauruscustomsettings.T_FORM_CUSTOM_WIDGET_MAP
3. implementing an entry-point for updating the custom widget map of a taurusform (and leaving the current tauruscustomsettings map as a deprecated fallback)

[reszelaz]

Many thanks for this in-depth analysis. I have not thought about the scenario of upgrading taurus only and in this case you are right - users will lost the PoolChannelTV (PCTV) in favor of TaurusValue for 0D and pseudo counters.

However I don't see it terrible for the following two reasons:

1. The main difference between these two widgets is that the old PCTV allows you to see at the first sight the Value attribute with a label widget. It is not very usefull cause to make it change you need to run a macro e.g. either a ct or a scan. Then if you run the macro you already see the updated value on the macro output. Well, of course there are other less relevant features of this widget e.g. the Value label shows in the background the attribute's quality.
2. The tauruscustomsettings.T_FORM_CUSTOM_WIDGET_MAP is a custom settings, so one always has the possibility to recover the previous behavior by changing this setting.

From the 3 options that you identify I'm in favor of putting all the efforts on 3. I will start disucssion on this option in a separate issue, however I would not block integration of these PRs. What do you think?

[cpascual]

The main difference between these two widgets is that the old PCTV allows you to see at the first sight the Value attribute with a label widget

Exactly, but I think that that is annoying for someone who already has a working GUI and it suddenly it changes its appearance after updating taurus (again, consider py2 users, which are still supported in taurus)

The tauruscustomsettings.T_FORM_CUSTOM_WIDGET_MAP is a custom settings, so one always has the possibility to recover the previous behavior by changing this setting.

But you realize that this works also as an argument against doing this change, right? ;)

I would not block integration of these PRs

Actually, I think it makes sense to block it. I think it is such a small change that it is not worth breaking taurus even if slightly and/or temporaly.

I'll do a PR ASAP (today) with the entry-point solution. And, in order to speed-up things, I can even advance the API:

in sardana you will be required to register the following:

entry_points = {
    "taurus.qt.qtgui.taurusform.custom_widget_map": ["<name> = <mod>:<dict>"],
    (...)
}

where <name> can be any string (I suggest sardana), and <mod>:<dict> points to a dictionary defined in some sardana submodule with the same syntax as T_FORM_CUSTOM_WIDGET_MAP and containing only the entries handled by sardana widgets (for example,sardana.sardanacustomsettings:T_FORM_CUSTOM_WIDGET_MAP)

[reszelaz]

I think there is no need to maintain this PR opened since the entry_point solution fixes this.
Thanks for the review!