Skip to content

To fix #3277#3280

Merged
vfdev-5 merged 8 commits intopytorch:masterfrom
puhuk:3277
Sep 9, 2024
Merged

To fix #3277#3280
vfdev-5 merged 8 commits intopytorch:masterfrom
puhuk:3277

Conversation

@puhuk
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@RMeli RMeli mentioned this pull request Feb 27, 2025
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