-
Notifications
You must be signed in to change notification settings - Fork 26
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
Several minor bugs in plot_trajectory
#417
Comments
I'm probably the one to blame fro the 2nd bug, because I encouraged relying on |
Thanks @niksirbi. Used your suggestions and double checked the plot_occupancy function that's coming with #403 but this doesn't seem to have the issues. I did not see |
I am currently working on updating the polar coordinate example #317 , and I also want to tackle #410 in combination with #415, which should take care of showcasing the occupancy plot. So don't worry about it. |
This issue highlights two related problems with the current
plot_trajectory
function: ambiguous or incorrect plot titles, and mismatched time/space units. Some of this may be also relevant for theplot_occupancy
function that's coming with #403.1. Misleading Titles
When plotting multiple trajectories on the same axis (as demonstrated here), the final plot title is determined by the last individual plotted. This can lead to confusion when multiple trajectories are shown.
In certain other edge cases, the title ends up displaying
"Trajectory of None"
.Proposed Fix
Rather than trying to infer a suitable title based on the plotted data, we should either:
"Trajectory"
).Users can always override the title themselves if needed.
2. Erroneous Time/Space Units
The
plot_trajectory()
function attempts to readspace_unit
andtime_unit
attributes directly from theposition
DataArray
. However, these attributes are only set at theDataset
level, meaningplot_trajectory()
defaults to"pixels"
and"frames"
even if the actual units are, for example,"meters"
and"seconds"
.Possible Solutions
Pass the entire
Dataset
This ensures that plotting functions have access to dataset-level attributes. However, it complicates matters when a dataset contains multiple variables (e.g.,
position
,confidence
) and forces users to carry unneeded data.Inherit attributes in the
DataArray
On dataset creation or file loading, propagate relevant attributes (e.g.,
time_unit
,space_unit
) to eachDataArray
. This requires deeper changes in the I/O layer and consideration of broader package implications.Stop attempting to infer units
As a quick hotfix, simply label plot axes as
"x"
,"y"
, and"Time"
without specifying units. This avoids misleading labels until we have a more comprehensive solution.Recommendation
I propose implementing Option 3 for now, allowing a functional v0.0.24 release. A more complete fix (Option 2) can be pursued later once we have time to address the underlying I/O and attribute-inheritance logic.
The text was updated successfully, but these errors were encountered: