Skip to content

Conversation

@johnwalz97
Copy link
Contributor

Internal Notes for Reviewers

The newly added raw data feature for tests is not currently supported when running comparison tests (i.e. using an input_grid or param_grid). For this initial solution, I am just appending the raw data elements to a list.

External Release Notes

@johnwalz97 johnwalz97 requested a review from juanmleng January 29, 2025 16:48
@johnwalz97 johnwalz97 added enhancement New feature or request internal Not to be externalized in the release notes labels Jan 29, 2025
Copy link
Contributor

@juanmleng juanmleng left a comment

Choose a reason for hiding this comment

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

Looks great, John! Thanks for addressing this. I just left a suggestion to handle cases where the tests don´t return raw data.

@johnwalz97 johnwalz97 merged commit 732bd0f into main Jan 31, 2025
5 checks passed
@johnwalz97 johnwalz97 deleted the john6797/sc-8259/support-raw-data-aggregation-in-comparison branch January 31, 2025 16:20
@github-actions
Copy link
Contributor

PR Summary

This pull request introduces enhancements to the validmind library, specifically focusing on the combination of test results. The key changes include:

  1. Addition of _combine_raw_data Function: A new function _combine_raw_data is introduced to handle the combination of RawData objects from a list of TestResult instances. This function ensures that all RawData objects have the same attributes and combines them into a single RawData object.

  2. Modification of combine_results Function: The combine_results function is updated to include the combination of RawData objects if they are present in the test results. This is achieved by appending the combined RawData to the combined_outputs list.

  3. Adjustment in run.py: The outputs parameter in the build_test_result function call is adjusted to directly use combined_outputs without converting it to a tuple, as it is already returned as a tuple from combine_results.

  4. Notebook Path Update: The path for one of the notebooks in scripts/run_e2e_notebooks.py is updated to reflect a new file name.

These changes aim to enhance the functionality of the result combination process by supporting the aggregation of raw data, which is crucial for comprehensive test result analysis.

Test Suggestions

  • Test the _combine_raw_data function with a list of TestResult objects having consistent RawData attributes to ensure correct combination.
  • Test the _combine_raw_data function with a list of TestResult objects having inconsistent RawData attributes to verify that a ValueError is raised.
  • Verify that the combine_results function correctly integrates the combined RawData into the combined_outputs.
  • Run end-to-end tests to ensure that the changes do not affect the overall functionality of the test result processing.
  • Test the updated notebook path in scripts/run_e2e_notebooks.py to ensure it points to a valid file.

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

Labels

enhancement New feature or request internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants