Skip to content

Fix codecov in tests for array api in pairwise metrics#29036

Merged
betatim merged 1 commit intoscikit-learn:mainfrom
OmarManzoor:fix_codecov
May 17, 2024
Merged

Fix codecov in tests for array api in pairwise metrics#29036
betatim merged 1 commit intoscikit-learn:mainfrom
OmarManzoor:fix_codecov

Conversation

@OmarManzoor
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Follow up of #29014

What does this implement/fix? Explain your changes.

Tried adding the condition which codecov reported as partially checked.

Any other comments?

CC: @ogrisel Do we need to add this or can we ignore?

@github-actions
Copy link
Copy Markdown

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: ff346a4. Link to the linter CI: here

Copy link
Copy Markdown
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

I think it might help catch weird bugs, so let's test this case.

@betatim betatim merged commit 48669a5 into scikit-learn:main May 17, 2024
Comment on lines +1900 to +1902
check_array_api_metric(
metric, array_namespace, device, dtype_name, a_np=X_np, b_np=Y_np
)
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.

Hi all, just realised that we are not passing the metric_kwargs to the function (also in the function call below). I think it should be:

check_array_api_metric(
             metric, array_namespace, device, dtype_name, a_np=X_np, b_np=Y_np, ** metric_kwargs
         )

should I make another PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ahrg! Yes please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I apologize for missing this.

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.

I also missed it in the previous PR 😇

See fix here: #29045

@jeremiedbb jeremiedbb mentioned this pull request May 20, 2024
14 tasks
@OmarManzoor OmarManzoor deleted the fix_codecov branch May 20, 2024 11:34
@jeremiedbb jeremiedbb mentioned this pull request Jul 2, 2024
11 tasks
@github-staff github-staff deleted a comment Sep 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants