Deprecate legacy Points{2|3}D and all their components
#2768
Conversation
This file contains 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
teh-cmc
added
🐍 Python API
teh-cmc
force-pushed
the
cmc/goodbye_old_point
branch
from
7ca7b0d to
33ac381
Compare
jleibs
force-pushed
the
jleibs/hope_legacy_shims
branch
from
8f05cdf to
25b60c3
Compare
teh-cmc
force-pushed
the
cmc/goodbye_old_point
branch
2 times, most recently
from
4e656a9 to
b3ba377
Compare
teh-cmc
force-pushed
the
cmc/points3d_arch
branch
2 times, most recently
from
c9587f7 to
77c1f92
Compare
teh-cmc
force-pushed
the
cmc/goodbye_old_point
branch
from
b3ba377 to
155b472
Compare
teh-cmc
changed the title
Point2D, use new Point2D all the way throughPoint{2|3}D, use new Point{2|3}D all the way through
teh-cmc
changed the title
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
teh-cmc
force-pushed
the
cmc/goodbye_old_point
branch
3 times, most recently
from
281c7d0 to
d4403d0
Compare
teh-cmc
changed the title
Points{2|3}D, use new Points{2|3}D all the way throughPoints{2|3}D and all their components
teh-cmc
force-pushed
the
cmc/goodbye_old_point
branch
from
edb792f to
d70ec97
Compare
teh-cmc
force-pushed
the
cmc/goodbye_old_point
branch
8 times, most recently
from
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
teh-cmc
force-pushed
the
cmc/goodbye_old_point
branch
from
96c777e to
d0f296f
Compare
Wumpf
approved these changes
Jul 28, 2023
| @@ -49,6 +49,7 @@ arrow2 = { workspace = true, features = [ | |||
| "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!() | ||
| } |
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: |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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