Skip to content

Add more qualx unit tests #1654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 30, 2025
Merged

Add more qualx unit tests #1654

merged 1 commit into from
Apr 30, 2025

Conversation

leewyang
Copy link
Collaborator

This PR adds more unit tests for qualx, and includes the following fixes found during testing:

  • sort Project fields in hash_util.py to allow for slight re-ordering of fields up to n_fields.
  • modify globbing of alignment CSV files to include the "current" file without any prefix.
  • add a configurable label_col argument to split_stratified.py.

@leewyang leewyang requested a review from amahussein April 28, 2025 20:35
@leewyang leewyang self-assigned this Apr 28, 2025
@github-actions github-actions bot added the user_tools Scope the wrapper module running CSP, QualX, and reports (python) label Apr 28, 2025
Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

Thanks @leewyang !
LGTME
Asking @sayedbilalbari to take a look as well as it can be a good opportunity to align his knowledge of the QualX tests after he worked on #1649

Copy link
Collaborator

@sayedbilalbari sayedbilalbari left a comment

Choose a reason for hiding this comment

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

Thanks @leewyang. I get a fair idea from the added tests as to what they are targeting. Perhaps a little more verbose comments would be helpful in understanding them better. Eg - test_compute_alignment_from_raw_features, is understandable from a perspective that I see that the alignment df is being verified for certain columns. Perhaps adding a refer -> , would be more helpful
LGTM

@leewyang
Copy link
Collaborator Author

Thanks @sayedbilalbari , will add more detailed comments in followups.

@leewyang leewyang merged commit 116b8ff into NVIDIA:dev Apr 30, 2025
15 checks passed
@leewyang leewyang deleted the qualx_unit_tests branch April 30, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user_tools Scope the wrapper module running CSP, QualX, and reports (python)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants