fix minor api load issues #459
Conversation
|
WalkthroughThe overall change focuses on enhancing the logic related to handling out-of-order timestamps and duplicate keys in the data index. The modification also includes a dynamic calculation of the initial repeat part column index based on the version of BONSAI_SLEAP. This adjustment allows for a more flexible and efficient handling of columns, improving the performance and maintainability of the code. Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (2)
- aeon/io/api.py (1)
- aeon/io/reader.py (1)
Review comments generated (2)
- Review: 2
- LGTM: 0
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| if not data.index.is_monotonic_increasing: | ||
| warnings.warn( | ||
| f"data index for {reader.pattern} contains out-of-order timestamps!", stacklevel=2 | ||
| ) | ||
| data = data.sort_index() | ||
| else: | ||
| elif data.index.has_duplicates: |
There was a problem hiding this comment.
The logic in the new hunk seems to be incorrect. The elif statement on line 153 will not execute if the first condition on line 148 is true, even if there are duplicate keys in the data index. This could lead to issues if the data index has both out-of-order timestamps and duplicate keys. I suggest changing the elif to an if so that both conditions can be checked independently.
- elif data.index.has_duplicates:
+ if data.index.has_duplicates:
warnings.warn(f"data index for {reader.pattern} contains duplicate keys!", stacklevel=2)
data = data[~data.index.duplicated(keep="first")]| if repeat_idxs: # drop x, y, and likelihood cols for repeat parts (skip first cols) | ||
| num_cols_skip = 5 if bonsai_sleap_v == BONSAI_SLEAP_V2 else 6 | ||
| init_rep_part_col_idx = (repeat_idxs - 1) * 3 + num_cols_skip |
There was a problem hiding this comment.
The check for repeat_idxs is not correct. In Python, an empty numpy array evaluates to True in a boolean context, which means the code inside the if block will execute even when repeat_idxs is empty. This could lead to unexpected behavior. You should explicitly check if repeat_idxs is not empty by using len(repeat_idxs) > 0.
- if repeat_idxs:
+ if len(repeat_idxs) > 0:
num_cols_skip = 5 if bonsai_sleap_v == BONSAI_SLEAP_V2 else 6
init_rep_part_col_idx = (repeat_idxs - 1) * 3 + num_cols_skip
|
@anayapouget thanks for these. Since they are changes to the low-level load API, do you mind submitting these to SainsburyWellcomeCentre/aeon_api? We are now tracking the low-level reader API in a separate repository. |
Summary by CodeRabbit