Refactor PointCloud

[cih9088]

Summary

Datumaro format and Datumaro binary format support two medias, namely Image and PointCloud.
The PointCloud should be able to digest functions, just like Image, for a single binary file export/import functionality, for example, Apache Arrow.

How to test

Checklist

• I have added unit tests to cover my changes.​
• I have added integration tests to cover my changes.​
• I have added the description of my changes into CHANGELOG.​
• I have updated the documentation accordingly

License

• I submit my code changes under the same MIT License that covers the project.
  Feel free to contact the maintainers if that's a concern.
• I have updated the license header for each file (see an example below).

# Copyright (C) 2023 Intel Corporation
#
# SPDX-License-Identifier: MIT

[codecov-commenter]

Codecov Report

Patch coverage: 72.07% and project coverage change: -0.02 ⚠️

Comparison is base (80d16dc) 78.90% compared to head (741b44c) 78.89%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #911      +/-   ##
===========================================
- Coverage    78.90%   78.89%   -0.02%     
===========================================
  Files          211      211              
  Lines        25059    25139      +80     
  Branches      5031     5035       +4     
===========================================
+ Hits         19774    19833      +59     
- Misses        4152     4171      +19     
- Partials      1133     1135       +2     

Flag	Coverage Δ
macos-11_Python-3.8	77.88% <72.07%> (-0.02%)	⬇️
ubuntu-20.04_Python-3.8	78.88% <72.07%> (-0.02%)	⬇️
windows-2019_Python-3.8	78.82% <72.07%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
datumaro/plugins/data_formats/kitti_raw/base.py	84.44% <ø> (ø)
...tumaro/plugins/data_formats/sly_pointcloud/base.py	88.99% <ø> (ø)
...ugins/data_formats/datumaro_binary/mapper/media.py	86.66% <50.00%> (-1.71%)	⬇️
datumaro/components/media.py	81.16% <72.44%> (-2.30%)	⬇️
datumaro/components/exporter.py	86.29% <100.00%> (+2.21%)	⬆️
datumaro/plugins/data_formats/datumaro/base.py	93.66% <100.00%> (ø)
datumaro/plugins/data_formats/datumaro/exporter.py	94.91% <100.00%> (-0.03%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[cih9088]

@vinnamkim How about this approach using factory?

[vinnamkim]

@vinnamkim How about this approach using factory?

LGTM. However, I think that it would be better explicitly to block Media from being created from its constructor. I mean a user must choose PointCloud.from_file() or PointCloud.from_data() to create PointCloud and PointCloud() is not allowed. Or, we can add warning for a user not to use the constructor directly. What do you think?

[cih9088]

LGTM. However, I think that it would be better explicitly to block Media from being created from its constructor. I mean a user must choose PointCloud.from_file() or PointCloud.from_data() to create PointCloud and PointCloud() is not allowed. Or, we can add warning for a user not to use the constructor directly. What do you think?

I thought the same thing and added assertion before your comment. What a coincidence.
How about now? If you agreed with this refactoring, I would propagate it to all other medias.

[vinnamkim]

LGTM. However, I think that it would be better explicitly to block Media from being created from its constructor. I mean a user must choose PointCloud.from_file() or PointCloud.from_data() to create PointCloud and PointCloud() is not allowed. Or, we can add warning for a user not to use the constructor directly. What do you think?

I thought the same thing and added assertion before your comment. What a coincidence. How about now? If you agreed with this refactoring, I would propagate it to all other medias.

LGTM, but that expansion would be done in the next PR not to make a big one here.

[cih9088]

LGTM, but that expansion would be done in the next PR not to make a big one here.

Yes, of course. I will make separate PRs for each one. Thank you for your comments. Will ping you when the PR is ready.

[vinnamkim]

LGTM.