strax icon indicating copy to clipboard operation
strax copied to clipboard

Make all dtypes immutable

Open WenzDaniel opened this issue 4 years ago • 1 comments

Minor thing I have not thought of and which I just noticed the hard way. Imagine the following: a user works on a new plugin and would like to use the interval dtype with some additional fields. So he/she does the following :

class MyPlugin(strax.Plugin)
    my_dtype = strax.inetrval_dtype
    my_dtype += new_fields
    dtype = my_dtype

Luckily this will lead to an error since the user will most likely use the same lines also in compute. So the new_fields will be added twice.

Below is a MWE:

import strax
import numpy as np

class MWE(strax.Plugin):
    """
    """
    __version__ = '0.0.1'

    parallel = 'process'
    depends_on = 'raw_records'
    compressor = 'lz4'
    rechunk_on_save = True

    provides = 'MWE'
    data_kind = 'MWE'
    
    MWE_dtype = strax.interval_dtype
    MWE_dtype += [(('Area field', 'area'), np.float32)]
    dtype = MWE_dtype
        
    def compute(self, raw_records):
        MWE_dtype_2 = strax.interval_dtype
        MWE_dtype_2 += [(('Area field', 'area'), np.float32)] # <-- will have the new fields twice
        
        res = np.zeros(1, dtype=MWE_dtype_2)
        res['time'] = raw_records[0]['time']
        res['dt'] = raw_records[0]['dt']
        res['length'] = raw_records[0]['length']
        
        return res  
    
strax.interval_dtype

So I would propose to change all dtype lists which we export into tuples.

WenzDaniel avatar May 05 '20 14:05 WenzDaniel

I think we could easily fix this by changing all the dtypes in strax to tuples indeed. The user would have to e.g. list(strax.interval_dtype) in case the user would like to add fields here.

This is quite simple but I can imagine that we're going to upset a lot of people by coldly changing this, custom plugins wouldn't work etc.

JoranAngevaare avatar Oct 30 '21 14:10 JoranAngevaare