mm2txt and mm2ply: Add new option --ignore-missing-fields#35
mm2txt and mm2ply: Add new option --ignore-missing-fields#35
Conversation
📝 WalkthroughWalkthroughAdded a CLI switch Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/source/app_mm2txt.rst (1)
125-126:⚠️ Potential issue | 🟡 MinorDocumentation inconsistency with the new
--ignore-missing-fieldsoption.Line 125 states "All specified fields must exist in the point cloud, or an error will be raised" but this is no longer strictly true when
--ignore-missing-fieldsis used. Consider updating this section to mention the exception.📝 Suggested documentation update
-- All specified fields must exist in the point cloud, or an error will be raised +- All specified fields must exist in the point cloud, or an error will be raised (unless ``--ignore-missing-fields`` is used, in which case missing fields produce a warning and are padded with zeros)docs/source/app_mm2ply.rst (1)
105-106:⚠️ Potential issue | 🟡 MinorDocumentation inconsistency with the new
--ignore-missing-fieldsoption.Line 105 states "All specified fields must exist in the point cloud, or an error will be raised" but this contradicts the new
--ignore-missing-fieldsbehavior. Consider updating for consistency.📝 Suggested documentation update
-- All specified fields must exist in the point cloud, or an error will be raised +- All specified fields must exist in the point cloud, or an error will be raised (unless ``--ignore-missing-fields`` is used, in which case missing fields produce a warning and are padded with zeros)
🤖 Fix all issues with AI agents
In `@apps/mm2ply/main.cpp`:
- Around line 481-486: Selected missing-fields cause saveToPly() to throw
because selectedFields still holds names not present in the layer even when
argIgnoreMissingFields.isSet(); fix by either filtering missing names out of
selectedFields before calling saveToPly() or by updating saveToPly() to tolerate
unknown fields (emit "0" like saveToTxt()). Concretely: when
argIgnoreMissingFields.isSet(), iterate the layer's available field names and
remove any selectedFields entries that are absent prior to invoking saveToPly(),
and apply the same filtering at the other call site noted (around the 271-274
usage); alternatively, modify saveToPly() to detect missing accessors and output
zeros for those fields (mirroring saveToTxt()) so it will not throw.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #35 +/- ##
========================================
Coverage 77.03% 77.03%
========================================
Files 179 179
Lines 9526 9526
Branches 917 917
========================================
Hits 7338 7338
Misses 2188 2188 🚀 New features to boost your workflow:
|
ceb0743 to
fd627a2
Compare
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.