Check duplicate `depends_on` of `Plugin`

yuema137 commented 2 months ago

Solve error #846.

Explanation

Basically what happened was:

In plugin.py, the chunks are fetched for each self.depends_on and stored in a dictionary: https://github.com/AxFoundation/strax/blob/2218e78722cb64cdedb17e255752b7857c5f229b/strax/plugins/plugin.py#L458-L493
Therefore, when there are duplicate items in self.depends_on , the later one overwrites the previous one
The later one cannot get anything, because the fetch is done in this way: https://github.com/AxFoundation/strax/blob/2218e78722cb64cdedb17e255752b7857c5f229b/strax/plugins/plugin.py#L467-L468 As the data is already read and split by the previous fetch, the later one can only get an empty result
So finally both ones are empty

Solution

The fix is simple, which is to remove the duplicate items in self.depends_on of the Plugin class.

coveralls commented 2 months ago

coverage: 90.437% (+0.007%) from 90.43% when pulling 94248d477a297c20c2b35136c148bf4d35ccd91e on fix_zero_length_repeated into 529886d2d9dccbe9e313347de77eb681fafdaaae on master.

yuema137 commented 2 months ago

@dachengx In order to pass the pre-commit check I had to format it with black & flake. I think it happens now and then when we make PR for the files that were not checked previously. Should we do a through cleaning for strax?

dachengx commented 2 months ago

This PR was initially aimed to solve https://github.com/AxFoundation/strax/issues/846, but replaced by https://github.com/AxFoundation/strax/pull/860.

But this PR reminds us that we need to make sure that the depends_on is unique. I will change this PR to a safe guard of Plugin.

AxFoundation / strax