Skip to content

Conversation

@Yusufibin
Copy link

@Yusufibin Yusufibin commented Nov 2, 2025

Main Changes

  • Code Quality: Added check for empty items and tests for edge cases (empty files, invalid inputs).

### Tests

  • All existing tests pass.

  • New tests added for edge cases.

  • Verified backward compatibility.

@danielfromearth
Copy link
Collaborator

Hi @Yusufibin, thanks for your interest in this repository and for sharing these proposed code changes!

In particular, I believe the following three changes will be improvements to the robustness of the batchee package:

  1. in batchee/harmony/util.py
    if not input_items:
        return []
  1. in ‎tests/test_filename_grouping.py‎
def test_empty_list():
    results = get_batch_indices([])
    assert results == []
  1. in ‎tests/test_filename_grouping.py‎
def test_invalid_filenames():
    invalid_filenames = ["invalid.nc", "TEMPO_NO2_L2_V03_20240731.nc", "random_file.txt"]
    results = get_batch_indices(invalid_filenames)
    assert results == []

I believe the other changes will add unnecessary complexity. If you are able to remove the other changes, we should be able to then merge these three above changes into the develop branch.

@danielfromearth danielfromearth added the enhancement New feature or request label Nov 7, 2025
@Yusufibin
Copy link
Author

Hello @danielfromearth ,
Thank you for your feedback on the pull request. I reviewed the code and removed the other changes to avoid unnecessary complexity, keeping only the three approved improvements to enhance the robustness of the batchee package:

  1. In batchee/harmony/util.py: Added if not input_items: return [] in the _get_output_bounding_box function to handle empty lists.
  2. In tests/test_filename_grouping.py: Added the test_empty_list test to verify behavior with an empty list.
  3. In tests/test_filename_grouping.py: Added the test_invalid_filenames test to verify behavior with invalid filenames.

All other changes (URL validation, performance optimizations, output formats, high-volume testing, etc.) have been removed. The tests pass successfully, confirming that the code remains functional.

The pull request is now ready for potential merging into the develop branch.

Copy link
Collaborator

@danielfromearth danielfromearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yusufibin, this looks good now. Thank you for making the updates!

There's one line here that I think should be reverted to how it was before, but otherwise it will be ready to go (pending new CHANGELOG entry and tests passing).

@Yusufibin
Copy link
Author

@danielfromearth Done

@danielfromearth
Copy link
Collaborator

pre-commit.ci autofix

Added comment to clarify the purpose of the day_and_scans list.
@danielfromearth
Copy link
Collaborator

pre-commit.ci autofix

@danielfromearth danielfromearth merged commit e98593b into nasa:develop Jan 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants