Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timedelta's should work with units #379

Closed
d-chambers opened this issue May 20, 2024 · 3 comments
Closed

timedelta's should work with units #379

d-chambers opened this issue May 20, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@d-chambers
Copy link
Contributor

Description

Both numpy and python time deltas should be recognized as time units.

For example, this currently fails:

import dascore as dc

patch = dc.get_example_patch()

time = dc.to_timedelta64(2)
patch.taper(time=time)

It should recognize time as a time.

Versions

  • OS [e.g. Ubuntu 20.04]:
  • DasCore Version [e.g. 0.0.5]:
  • Python Version [e.g. 3.10]:
@d-chambers d-chambers added the bug Something isn't working label May 20, 2024
@ahmadtourei
Copy link
Collaborator

Hey @d-chambers, I just ran this, and it works with applying units:

import dascore as dc
from dascore.units import s

time = dc.to_timedelta64(2)
pa_tapered_1 = pa.taper(time=time*s)
pa_tapered_2 = pa.taper(time=2*s)
assert p_tapered_1 == p_tapered_2
assert np.array_equal(pa_tapered_1.data, pa_tapered_2.data), "The arrays are not equal."

It's expected to use units with taper since kwarg is the percentage of the total length of the dimension or absolute units.

I also tried rolling (without specifying units) and got the same result:

import dascore as dc

time = dc.to_timedelta64(2)
mean_pa_1 = pa.rolling(time=time, step=0.5).mean()
mean_pa_2 = pa.rolling(time=2, step=0.5).mean()
assert np.array_equal(mean_pa_1.dropna("time").data, mean_pa_2.dropna("time").data), "The arrays are not equal."

@d-chambers
Copy link
Contributor Author

Hey @d-chambers, I just ran this, and it works with applying units:

Interesting. Yes, I think the current issue is when a single timedelta64 is passed without applying units to it. Timedelta64 is a bit of a weird case in that it already has units implied.

@d-chambers
Copy link
Contributor Author

closed by #422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants