Skip to content

fix: hide overwrite log - #1535

Merged
SAKURA-CAT merged 2 commits into
legacyfrom
fix/hide-overwrite
Apr 9, 2026
Merged

SAKURA-CAT merged 2 commits into
legacyfrom
fix/hide-overwrite

Conversation

@Nexisato

@Nexisato Nexisato commented Apr 9, 2026 •

Copy link
Copy Markdown
Collaborator

Description

  • Due to the House performance, hide overwrite functionality temporarily

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request modifies the _add function in swanlab/data/run/exp.py to prevent overwriting existing data steps. Instead of allowing overwrites, the function now returns an error if a duplicate step is detected. A review comment suggests removing the now-unreachable commented-out code block to improve code clarity and maintainability.

Comment thread swanlab/data/run/exp.py
Comment on lines +146 to +150
# # 允许 overwrite,但区分显式指定和隐式的碰撞
# if explicit_step:
# swanlog.debug(f"Step {step} on key {key} already exists, overwriting.")
# else:
# swanlog.warning(f"Implicit step {step} on key {key} resolved as overwrite, but expected to append.")

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.

medium

This block of commented-out code is unreachable due to the return statement on the preceding line. It should be removed to improve code clarity and maintainability. Version control can be used to track the history of this logic if it needs to be restored in the future.

@Nexisato Nexisato self-assigned this Apr 9, 2026
@SAKURA-CAT
SAKURA-CAT merged commit 2dba3e2 into legacy Apr 9, 2026
5 checks passed
@SAKURA-CAT
SAKURA-CAT deleted the fix/hide-overwrite branch April 9, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants