-
Notifications
You must be signed in to change notification settings - Fork 904
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
Fix/Make DTW alignment plot compatible with RangeIndex #1880
Fix/Make DTW alignment plot compatible with RangeIndex #1880
Conversation
Modify the DTWAlignment.plot_alignment() method so it works with both DateTime & RangeIndex time dimensions
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #1880 +/- ##
==========================================
- Coverage 93.95% 93.92% -0.03%
==========================================
Files 125 125
Lines 11738 11729 -9
==========================================
- Hits 11028 11017 -11
- Misses 710 712 +2
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AhmetZamanis and thanks a lot for this fix :) 🚀
I only had two minor suggestions.
darts/dataprocessing/dtw/_plot.py
Outdated
@@ -151,18 +151,19 @@ def plot_alignment( | |||
time_dim1 = series1._time_dim |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you change these to series1.time_dim
and series2.time_dim
?
darts/dataprocessing/dtw/_plot.py
Outdated
x_coords = np.zeros(n * 3, dtype=xa1[time_dim1].dtype) | ||
y_coords = np.empty(n * 3, dtype=np.float64) | ||
|
||
x_coords[0::3] = x_coords1 | ||
x_coords[1::3] = x_coords2 | ||
x_coords[2::3] = np.datetime64("NaT") | ||
if x_coords.dtype == "datetime64[s]": | ||
x_coords[2::3] = np.datetime64("NaT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change lines 160-166 to the following?
The reason why x_coords[2::3] = np.nan
failed for integer indexed series, is because only float arrays (not integer) allow NaNs
if series1.has_datetime_index:
x_dtype = xa1[time_dim1].dtype
x_nan = np.datetime64("NaT")
else:
x_dtype = np.float64
x_nan = np.nan
x_coords = np.empty(n * 3, dtype=x_dtype)
y_coords = np.empty(n * 3, dtype=np.float64)
x_coords[0::3] = x_coords1
x_coords[1::3] = x_coords2
x_coords[2::3] = x_nan
Thanks @dennisbader, I made the changes you required. Let me know if there is any issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thanks a lot for your contribution @AhmetZamanis 🚀 💯
Fixes issue #1871 by modifying the
DTWAlignment.plot_alignment()
method, so it works with bothDateTimeIndex
&RangeIndex
time dimensions.Summary
Currently,
plot_alignment()
converts the x-axis coordinates for the alignment lines into DateTime (lines 154-155). The PR modifies this so the x-axis coordinates match the datatype of the series' time dimensions (DateTime or int).I also replaced
np.empty
withnp.zeros
(line 160), as doingx_coords[2::3] = np.nan
raises an error (not sure why), but leavingx_coords[2::3]
as all zeroes seems to work for RangeIndex time dimensions.Other Information
I tested this fix on the same series with both DateTime and RangeIndex time dimensions, and they produced the same plots.