Option to have a Tsd as an index

[vigji]

When working with Tsds, I might sometimes write, eg for filtering:

some_tsd = Tsd(...)
some_tsd.d[some_tsd.d > 500] = np.nan

I think it would be nice and not confusing for how the class works atm (very thorough feature equivalence with np.ndarray) having instead:

some_tsd = Tsd(...)
some_tsd[some_tsd > 500] = np.nan

Just by changing the setter and getter for the Tsd.

Would something like this make sense? This is just a draft for Tsd, it could be extended to other classes maybe although I never encountered the need.

[gviejo]

i think there is a conflit with the previous PR

[gviejo]

I think it's good and I would keep it only for time series.

[vigji]

Everything should have been addressed now, let me know if there's anything else I should fix

[gviejo]

That's great. I would still add one test for it.

[vigji]

Excellent point, adding tests I actually realised I did not set the behavior for setitem in TsdFrame.

This should be now be good to go!

missing for TsdTensor

[gviejo]

It's missing for TsdTensor I think?

[gviejo]

On line 732

    if isinstance(key, BaseTsd):
        key = key.d

[vigji]

Oh sorry maybe I misunderstood one of your early messages - do you thing this should be implemented for Tsdtensors as well?

[vigji]

Or you mean I should exclude tensors?

[gviejo]

Yes I would implement it for TsdTensor too

[vigji]

I'll come back to this soon if still unaddressed!

Since I am at it, I'll ask a slightly related but separate question: why is TsIndex not a Ts but a np.array? Is there some actual reason or it is just historical?

[gviejo]

Basically TsIndex is like pd.Index in pandas. Ts is the user facing module. TsIndex is the non-user facing module which hold the timestamps and timestamps methods across all modules with time.

[vigji]

This should now good to go!

I did a minor refactoring of test classes for readability, hopefully that's fine. Also, I'd love to do a separate PR for tests cleanup if that is good for you!

should fixed now

[gviejo]

Looks good a few changes only

[BalzaniEdoardo]

Thanks Luigi! I did not go through the tests yet, the main code looks good to me, but I added a couple of suggestions;

[vigji]

Let mw know if anything else is needed here!

(In the test refactoring, move test generated files to temporary folders would be very useful to avoid spurious commits)

[gviejo]

Thanks I will look into it as soon as possible.

[gviejo]

It's almost good. Is it possible to add this for all time series :

            if not all(
                np.issubdtype(k.dtype, np.bool_) if isinstance(k, Tsd) else True
                for k in key
            ):
                raise ValueError(
                    "When indexing with a Tsd, it must contain boolean values"
                )

I should only be boolean indexing with another Tsd otherwise it doesn't make sense to index with integers because it means you are breaking the relationship between the time index and the corresponding values. Somehow it's similar to shuffling but it should be a proper method in the shuffling module.

[vigji]

Ok, done it! I also change a bit the logic to bit more interpretable, and fixed the tests accordingly

[vigji]

Should have fixed stuff and rebased on dev head. Would be great if we can merge so that I won't have to resolve more conflicts in the future!

[gviejo]

Looks good thank you Luigi