Skip to content

Improve legacy H5 format test coverage#22665

Open
andersendsa wants to merge 2 commits intokeras-team:masterfrom
andersendsa:improve-coverage
Open

Improve legacy H5 format test coverage#22665
andersendsa wants to merge 2 commits intokeras-team:masterfrom
andersendsa:improve-coverage

Conversation

@andersendsa
Copy link
Copy Markdown
Contributor

@andersendsa andersendsa commented Apr 11, 2026

Description

  • Added model factories for Conv2D, RNN, Bidirectional, and Embedding layers.
  • Expanded LegacyH5WeightsTest, LegacyH5WholeModelTest, and LegacyH5BackwardsCompatTest with these new models.

Contributor Agreement

Please check all boxes below before submitting your PR for review:

  • I am a human, and not a bot.
  • I will be responsible for responding to review comments in a timely manner.
  • I will work with the maintainers to push this PR forward until submission.

Note: Failing to adhere to this agreement may result in your future PRs no longer being reviewed.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request expands the test suite for legacy H5 format saving by adding helper functions and test cases for convolutional, RNN, and embedding models. The review feedback highlights formatting issues such as missing blank lines between methods and trailing whitespace on empty lines.

tf_keras_model = get_subclassed_model(tf_keras)
ref_input = np.random.random((2, 3))
self._check_reloading_weights(ref_input, model, tf_keras_model)
def test_conv_model_weights(self):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Missing blank line between methods. According to PEP 8 and Keras coding standards, method definitions inside a class should be separated by a single blank line to improve code readability and maintain consistency with the rest of the file.

Suggested change
def test_conv_model_weights(self):
def test_conv_model_weights(self):
References
  1. Following Python conventions (PEP 8), methods should be separated by a single blank line. (link)

model = get_functional_model(keras)
ref_input = np.random.random((2, 3))
self._check_reloading_model(ref_input, model)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Trailing whitespace detected. This line contains indentation spaces but no code. Blank lines should be kept truly empty to avoid unnecessary whitespace churn and adhere to clean coding practices.

References
  1. Avoid trailing whitespace to maintain clean code formatting. (link)

legacy_h5_format.save_model_to_hdf5(model, temp_filepath)
legacy_h5_format.load_model_from_hdf5(temp_filepath)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Trailing whitespace detected. This line should be a clean blank line without any spaces.

References
  1. Avoid trailing whitespace to maintain clean code formatting. (link)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.11%. Comparing base (75d7bea) to head (375cff4).

❗ There is a different number of reports uploaded between BASE (75d7bea) and HEAD (375cff4). Click for more details.

HEAD has 6 uploads less than BASE
Flag BASE (75d7bea) HEAD (375cff4)
keras 5 2
keras-tensorflow 1 0
keras-torch 1 0
keras-jax 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #22665       +/-   ##
===========================================
- Coverage   83.15%   64.11%   -19.05%     
===========================================
  Files         596      596               
  Lines       68858    68858               
  Branches    10774    10774               
===========================================
- Hits        57259    44147    -13112     
- Misses       8783    22142    +13359     
+ Partials     2816     2569      -247     
Flag Coverage Δ
keras 64.10% <ø> (-18.87%) ⬇️
keras-jax ?
keras-numpy 54.88% <ø> (-0.01%) ⬇️
keras-openvino 59.43% <ø> (ø)
keras-tensorflow ?
keras-torch ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants