Skip to content

Commit

Permalink
Update docs to match exclusive end_time implementation (#1286)
Browse files Browse the repository at this point in the history
### Changelog

None

### Docs

* Updated the Python docstrings for the MCAP readers to label the
`end_time` parameter as an exclusive bound
* The documentation on [this
page](https://mcap.dev/docs/python/mcap-apidoc/mcap.reader#mcap.reader.McapReader.iter_decoded_messages)
will also be updated

### Description

According to the documentation
[here](https://mcap.dev/docs/python/mcap-apidoc/mcap.reader#mcap.reader.McapReader.iter_decoded_messages),
the iterator should exclude messages that were logged before
`start_time` or after `end_time` (i.e. both inclusive bounds). However,
the implementation also excludes messages published at the same time as
`end_time` (making it an exclusive bound). The documentation has been
updated to match the implementation.

This discrepancy in the Python package is only happening in
`python/mcap/mcap/reader.py` where `iter_message()` and
`iter_decoded_message()` are defined. In all of the other files where
those functions are called, `end_time` is described as an exclusive
bound:
- `python/mcap-protobuf-support/mcap_protobuf/reader.py`
- `python/mcap-ros1-support/mcap_ros1/reader.py`
- `python/mcap-ros2-support/mcap_ros2/reader.py`

This motivated updating the documentation rather than modifying the
implementation (which would also have been a breaking change). In
addition, this follows the typical Python convention of using
inclusive-start and exclusive-end bounds (`list[start:end]`,
`array[start:end]`, `range(start, end)`, `numpy.arange(start, end)`,
etc...).

<table>
  <tr>
    <th></th><th>Before</th><th>After</th>
  </tr><tr>
    <td>Documentation</td>
    <td><code>end_time</code> is inclusive</td><td>
    <code>end_time</code> is exclusive</td>
  </tr><tr>
    <td>Implementation</td>
    <td><code>end_time</code> is exclusive</td>
    <td><code>end_time</code> is exclusive</td>
  </tr>
</table>


Fixes: FG-9583

---------

Co-authored-by: james-rms <[email protected]>
  • Loading branch information
rdumas01 and james-rms authored Dec 11, 2024
1 parent 212b873 commit 5db8c69
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions python/mcap/mcap/reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def _chunks_matching_topics(
:param summary: the summary of this MCAP.
:param topics: topics to match. If None, all chunk indices in the summary are returned.
:param start_time: if not None, messages from before this unix timestamp are not included.
:param end_time: if not None, messages from after this unix timestamp are not included.
:param end_time: if not None, messages at or after this unix timestamp are not included.
"""
out: List[ChunkIndex] = []
for chunk_index in summary.chunk_indexes:
Expand Down Expand Up @@ -138,8 +138,8 @@ def iter_messages(
:param topics: if not None, only messages from these topics will be returned.
:param start_time: an integer nanosecond timestamp. if provided, messages logged before this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged after this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged at or after
this timestamp are not included.
:param log_time_order: if True, messages will be yielded in ascending log time order. If
False, messages will be yielded in the order they appear in the MCAP file.
:param reverse: if both ``log_time_order`` and ``reverse`` are True, messages will be
Expand All @@ -160,8 +160,8 @@ def iter_decoded_messages(
:param topics: if not None, only messages from these topics will be returned.
:param start_time: an integer nanosecond timestamp. if provided, messages logged before this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged after this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged at or after
this timestamp are not included.
:param log_time_order: if True, messages will be yielded in ascending log time order. If
False, messages will be yielded in the order they appear in the MCAP file.
:param reverse: if both ``log_time_order`` and ``reverse`` are True, messages will be
Expand Down Expand Up @@ -274,8 +274,8 @@ def iter_messages(
:param topics: if not None, only messages from these topics will be returned.
:param start_time: an integer nanosecond timestamp. if provided, messages logged before this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged after this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged at or after
this timestamp are not included.
:param log_time_order: if True, messages will be yielded in ascending log time order. If
False, messages will be yielded in the order they appear in the MCAP file.
:param reverse: if both ``log_time_order`` and ``reverse`` are True, messages will be
Expand Down Expand Up @@ -476,8 +476,8 @@ def iter_messages(
:param topics: if not None, only messages from these topics will be returned.
:param start_time: an integer nanosecond timestamp. if provided, messages logged before this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged after this
timestamp are not included.
:param end_time: an integer nanosecond timestamp. if provided, messages logged at or after
this timestamp are not included.
:param log_time_order: if True, messages will be yielded in ascending log time order. If
False, messages will be yielded in the order they appear in the MCAP file.
:param reverse: if both ``log_time_order`` and ``reverse`` are True, messages will be
Expand Down

0 comments on commit 5db8c69

Please sign in to comment.