Deprecate legacy Points{2|3}D and all their components#2768
Merged
Conversation
7ca7b0d to
33ac381
Compare
8f05cdf to
25b60c3
Compare
4e656a9 to
b3ba377
Compare
c9587f7 to
77c1f92
Compare
b3ba377 to
155b472
Compare
Point2D, use new Point2D all the way throughPoint{2|3}D, use new Point{2|3}D all the way through
Point{2|3}D, use new Point{2|3}D all the way throughPoints{2|3}D, use new Points{2|3}D all the way through
281c7d0 to
d4403d0
Compare
Points{2|3}D, use new Points{2|3}D all the way throughPoints{2|3}D and all their components
edb792f to
d70ec97
Compare
b348b39 to
0bfbd10
Compare
This is fine because A) this won't ever be released and B) as [1] shows, nobody as ever used
```
rr.log_points("a/b/c", colors=[red, blue, blue])
```
since that simply never worked.
[1] https://github.com/rerun-io/rerun/blob/main/rerun_py/rerun_sdk/rerun/log/points.py#L225-L226
96c777e to
d0f296f
Compare
Wumpf
approved these changes
Jul 28, 2023
| "compute_concatenate", | ||
| ] } | ||
| anyhow.workspace = true | ||
| arrow2_convert.workspace = true |
Comment on lines
+51
to
+74
| fn try_to_arrow( | ||
| &self, | ||
| ) -> re_types::SerializationResult< | ||
| Vec<( | ||
| re_viewer::external::arrow2::datatypes::Field, | ||
| Box<dyn re_viewer::external::arrow2::array::Array>, | ||
| )>, | ||
| > { | ||
| unimplemented!() | ||
| } | ||
|
|
||
| fn try_from_arrow( | ||
| _data: impl IntoIterator< | ||
| Item = ( | ||
| re_viewer::external::arrow2::datatypes::Field, | ||
| Box<dyn re_viewer::external::arrow2::array::Array>, | ||
| ), | ||
| >, | ||
| ) -> re_viewer::external::re_types::DeserializationResult<Self> | ||
| where | ||
| Self: Sized, | ||
| { | ||
| unimplemented!() | ||
| } |
Member
There was a problem hiding this comment.
if we were able to compress the visual space of this somehow it wouldn't be too bad 🤔
|
|
||
|
|
||
| # NOTE: This has to live here for now, while we migrate to archetypes (circular experimental imports). | ||
| def splat() -> Any: |
Member
There was a problem hiding this comment.
would still appreciate calling this instance_key_splat to make it clear what we're splatting
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
points2dandpoints3dviews are now both implemented in terms of the new components (that was already the case for points2d).This PR removes all of these:
DrawOrderPoint2DPoint3DRadiusInstanceKeyand deprecates all of those:
ColorLabelClassIdKeypointIdThe reason these last four are merely deprecated rather than removed entirely is that the legacy
AnnotationContextdepends on them, and more specifically depends on their respective implementations ofarrow2-convert's traits.Still, they have been confined into tight corners as much as possible.
The deprecated type are prefixed with
Legacy, e.g.LegacyLabel.In particular,
ColorRGBAis nowLegacyColor.In Rust, the new components aren't hidden behind an
experimentalmodule anymore, instead they are transparently merged with the rest of the components.The new
Point2Dcomponent implements the oldLegacyComponenttrait to allow all of the oldre_querytest suites to continue working, until we remove them all soon.The legacy point logging APIs,
log_point&log_points, have been rewritten in terms of thePoints2D&Points3Darchetypes.In order to do so, I had to make their
positionargument mandatory, i.e. you can not write this anymore:rr.log_points("a/b/c", colors=[red, red, blue]).This is not a problem for two reasons:
cc @Wumpf, the custom view example got a bit more verbose and should get much much better once we tackle #2778. We could also just modify it to use an existing archetype instead (which would have to be either
Points2DorPoints3Dat the moment).Fixes #2790
What
Checklist