Skip to content
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

To fix #3277 #3280

Merged
merged 8 commits into from
Sep 9, 2024
Merged

To fix #3277 #3280

merged 8 commits into from
Sep 9, 2024

Conversation

puhuk
Copy link
Contributor

@puhuk puhuk commented Sep 4, 2024

Fixes #3277

Description:

To fix #3277 , replace _VALID_PARAM_AND_METRIC_NAMES to regular expression from mlflow

https://github.com/mlflow/mlflow/blob/d6625d6df85b61f3661ba90efc36f94511a5c23f/mlflow/utils/validation.py#L15C1-L15C60

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: handlers Core Handlers module label Sep 4, 2024
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Sep 4, 2024

Hey @puhuk , thanks for the PR, can you check what mlflow did with _VALID_PARAM_AND_METRIC_NAMES variable and whether we can't use its successor?

@puhuk
Copy link
Contributor Author

puhuk commented Sep 4, 2024

@vfdev-5 Thanks for the review!
It validated params and metric names whether only contain slashes and alphanumerics.
Now, it has replaced with below method and I updated with my PR.

https://github.com/mlflow/mlflow/blob/807b729a74c1e2ba1b5bfa51edefbbc9b0054fd7/mlflow/utils/validation.py#L109-L117

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @puhuk !

@vfdev-5 vfdev-5 enabled auto-merge (squash) September 9, 2024 21:41
@vfdev-5 vfdev-5 merged commit 653fb9e into pytorch:master Sep 9, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: handlers Core Handlers module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] Fix test issue with recent MLfow
2 participants