cherab
Class for managing lists holding same types
ModelManagers are now being used in several places as beam node and plasma node. Now the same could be use in the Zeeman lineshape models (PR #246 ) and also is used in the upcoming Thomson scattering (PR #219 ). Shouldn't the ModelManager class be added to tools, so it can be reused instead of defined every time? The type of the objects in the list could be a mandatory parameter of the init.
I agree that this might be handy. Although, in particular case of PR #246 the contents of the lists do not change after initialisation, and I like the use of autowrap_function1d() for the Function1D objects more than type checking in the ModelManager.
In their current forms the model managers can't be made into generic tools as the cython types cannot be dynamically changed. That said, I'm not sure why I wrote the managers in cython. There is probably scope for a clean up, but it isn't really a priority.
@vsnever and @CnlPepper I made a draft PR #273. This is what I had in mind when proposing to have some class replacing the ModelManagers. It is basically the same, the list handling methods are python and there are 2 cpdef methods for getting the list and its length for faster approach in cython.
Maybe I'm missing something here, but why do we need ModelManagers at all? What's wrong with a standard Python list? The existing ModelManager implementations seem to just be wrappers around Python lists anyway, and lists are fast to work with even in Cython. Even looping over the managers is going through the Python layer, as it uses the__iter__ special method. And Cython should type check each element that's retrieved for you if you specify the type of the variable you assign the element to first, as is already done in the beam and plasma objects.
Before we invest any time trying to better list, I'd like to know what benefit dedicated ModelManager classes have over a list. Perhaps getting rid of them entirely and reducing code complexity would actually be more useful.
This is actually quite a good comment. I think the only advantage except of checking the type of items on addition is the notifier.
The advantage of checking the type on addition can provide clearer debugging. The manager will tell clearly the user he/she tried to add wrong type. This might be useful for less skilled and experienced users. The type checks on addition can be added separately to plasma, beams, lasers, etc. but then you would be repeating what the manager does, wouldn't you?
The notifier could be useful for updating caches. I am not aware of any cherab class doing it now, but I can imagine it could be useful in the future. For example in the case of beams, you don't have to rebuild the whole geometry when models are changed, you only update the list in the material.