Skip to content

Conversation

@LINYV0719
Copy link

…sing return types

Description

The filename argument in JSONPersistedValue was annotated as str, but the implementation explicitly checks if self._filename is not None, implying it should be Optional[str]
This PR:
Changes filename: str to filename: Optional[str].
Adds missing return type hints (-> Any, -> None) for read and write methods.
Verified with python -m orbit.actions.new_best_metric_test.

Type of change

For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.

Note: Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update
  • TensorFlow 2 migration
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • A new research paper code implementation
  • Other (Specify)

Tests

I ran the existing unit tests to verify that the type hint changes didn't break any functionality.

Test Configuration:

OS: Windows
Python Version: Python 3.10
Command: python -m orbit.actions.new_best_metric_test
Result: All tests passed.

Checklist

@LINYV0719 LINYV0719 requested a review from a team as a code owner December 26, 2025 13:15
@google-cla
Copy link

google-cla bot commented Dec 26, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@LINYV0719 LINYV0719 force-pushed the fix/new-best-metric-types branch from 868e2d1 to b4f3334 Compare December 26, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant