Merged
Conversation
Introduce EChartsItem and make EChartsValue hold repeated items; regenerate Go/Python protobuf bindings. Move transform implementations from swanlab/sdk/internal/run/data/transforms to swanlab/sdk/internal/run/transforms and add normalize_media_input helper. Revise TransformType/TransformMediaType APIs (new abstract methods, build_metric_record, column_type, type, changed transform signatures) and simplify validation. Update RecordBuilder to dispatch on TransformMediaType and lists, return transformer classes for implicit column creation, and map MetricRecord -> media type more strictly. Update BackgroundConsumer, run APIs (log_text/log_scalar), Text/Scalar transforms, typings, imports and unit tests accordingly. Misc: remove obsolete data package init files and fix related imports.
Restructure and add metric protobuf schema and generated bindings: move protos from protos/swanlab/data/v1 to protos/swanlab/metric/*, add new column and data proto definitions (including media and scalar subtypes), and remove the old metric.proto. Generate Go protos under core/proto/swanlab/metric/... and Python protos under swanlab/proto/swanlab/metric/..., update record proto/pb files, and apply related minor updates to SDK transform and unit tests. This change provides the new metric/column/data schema (audio, image, video, text, echarts, scalar) and their language bindings.
Modify proto definitions (protos/swanlab/metric/column/v1/column.proto, protos/swanlab/metric/data/v1/data.proto, protos/swanlab/record/v1/record.proto) and regenerate protobuf outputs. Remove outdated generated files under core/proto/swanlab/data/v1 and swanlab/proto/swanlab/data/v1, and update regenerated Go and Python protobuf artifacts (pb.go, pb2.py, and .pyi) for metric/column, metric/data and record. Also apply a small update to swanlab/sdk/internal/run/transforms/text/__init__.py. Cleans up old artifacts and aligns generated code with the updated .proto definitions.
Update the module docstring to indicate that the ColumnType enum adapter also serves as a path mapping. Documentation-only change; no functional code modified.
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant refactoring of SwanLab's data and media handling infrastructure. The primary goal is to streamline the internal representation of metrics and columns using a more organized Protocol Buffer structure and to simplify the interaction between user input and these internal types via a new adapter layer. These changes improve the system's maintainability, consistency, and extensibility for future data types. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
这次重构极大地改善了 media 部分的逻辑和 Protobuf 定义。
主要亮点包括：
- 更清晰的 Protobuf 结构: 将
metric相关的定义移动到swanlab/metric目录下，并按column和data进行了组织，结构更加清晰。 - 引入适配器模式: 新增的
adapter模块使用BiMap来处理用户输入字符串与内部 Protobuf 枚举的映射，这是一个非常好的实践，增强了代码的可维护性和解耦性。 - 改进的数据转换流程:
TransformType的重构和RecordBuilder中对singledispatchmethod的使用，使得数据处理流程更加健壮和可扩展。 - 简化的媒体类型定义: 移除了媒体类型 Protobuf 定义中的
path字段，将路径管理责任上移到 SDK 层面，简化了消息结构。
总体来说，这是一次高质量的重构，显著提升了代码质量。我发现了一个潜在的 bug 和一个文档不一致的地方，请见具体的审查评论。
Introduce swanlab.sdk.internal.adapter.dirname and filename modules to centralize directory and filename conventions (constants for files, media, debug; metadata, config, requirements, conda and a run(run_id) helper). Export them from adapter.__init__.py and update RunContext to use these constants instead of hardcoded strings, add a debug_dir property, and use adapter.filename.run for the run file path. This centralizes naming conventions and reduces literal strings in the context code.
Change TransformType.transform from a staticmethod to an instance method with signature transform(self, *, key: str, step: int, data: str = None) and simplify content fallback to use data or an empty string. Remove the redundant type() classmethod. In RecordBuilder, rename adapter.column_type[col_type] to media_type_str and pass the original col_type (enum) to metrics.define_media while using media_type_str for the media directory path—aligning argument order and clarifying variable names.
Replace usages of unittest.mock.patch with pytest's monkeypatch across several tests to make mocking more consistent and robust. Key changes: - tests/benchmark/sdk/internal/run/bench_run_helper.py: switch to monkeypatch for suppressing console warnings in validate_key benchmarks. - tests/unit/sdk/cmd/login/test_login_e2e.py: migrate patches to monkeypatch, simulate prompt calls with a side-effect list, make host configurable (use fake.swanlab.cn in one test), and ensure client.exists/reset are monkeypatched. Adjusted calls to pass relogin where needed and assert saved .netrc content. - tests/unit/sdk/internal/pkg/fs/test_fs_dir.py: replace MagicMock sys.modules hack with simple module-like objects, convert time and TemporaryFile patches to monkeypatch-based replacements, and capture console warnings into a list for assertions. Clean up environment variable tests accordingly. - tests/unit/sdk/internal/pkg/fs/test_fs_write.py: replace os.replace patch with monkeypatch and validate atomic write rollback behavior. - tests/unit/sdk/internal/run/test_run_fmt.py: replace console patching with monkeypatch capturing console.warning calls, and adapt tests to avoid global patch use. Overall these changes modernize tests to use pytest fixtures, reduce reliance on context-managed patching, and make warning/assertion checks explicit and side-effect free.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
重构 媒体 部分的逻辑，并且简化了类型定义，现在我们通过 adapter 模块映射protobuf枚举与用户语义