sync cseslib4py #2
Merged
Conversation
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
审阅者指南此 PR 重构了核心 CSES API，以使用工厂方法进行加载和序列化，通过时间转换和 YAML 钩子增强了内部实用工具，将数据模型现代化为使用原始类型和验证器而非自定义枚举，并更新了示例和错误模块以保持一致性。 重构后的 CSES 核心 API 和模型类图classDiagram
class CSES {
+int version
+dict subjects
+Schedule schedules
+CSES()
+from_str(str content) CSES
+from_file(str fp) CSES
+to_yaml() str
+to_file(str fp, str mode)
+_gen_dict() dict
+__eq__(other)
}
class Subject {
+str name
+Optional[str] simplified_name
+Optional[str] teacher
+Optional[str] room
}
class Lesson {
+str subject
+datetime.time start_time
+datetime.time end_time
+serialize_time(time: datetime.time) str
}
class SingleDaySchedule {
+Literal[1,2,3,4,5,6,7] enable_day
+list~Lesson~ classes
+str name
+Literal['all','odd','even'] weeks
+is_enabled_on_week(int week) bool
+is_enabled_on_day(datetime.date start_day, datetime.date day) bool
}
class Schedule {
+list~SingleDaySchedule~ data
}
CSES --> Schedule : schedules
CSES --> Subject : subjects
Schedule --> SingleDaySchedule
SingleDaySchedule --> Lesson
Lesson --> Subject : subject (by name)
新的实用函数和日志记录器类图classDiagram
class utils {
+week_num(datetime.date, datetime.date) int
+ensure_time(str|int|datetime.time) datetime.time
+serialize_time(dumper, datetime.time) yaml.nodes.ScalarNode
+NoAliasDumper
+log
+repr_
}
class NoAliasDumper {
+ignore_aliases(data)
+increase_indent(flow, indentless)
}
utils --> NoAliasDumper
文件级变更
提示和命令与 Sourcery 交互
自定义你的体验访问你的仪表盘以：
获取帮助Original review guide in EnglishReviewer's GuideThis PR refactors the core CSES API to use factory methods for loading and serialization, enhances internal utilities with time conversion and YAML hooks, modernizes data models to use primitive types and validators instead of custom enums, and updates example and error modules for consistency. Class diagram for refactored CSES core API and modelsclassDiagram
class CSES {
+int version
+dict subjects
+Schedule schedules
+CSES()
+from_str(str content) CSES
+from_file(str fp) CSES
+to_yaml() str
+to_file(str fp, str mode)
+_gen_dict() dict
+__eq__(other)
}
class Subject {
+str name
+Optional[str] simplified_name
+Optional[str] teacher
+Optional[str] room
}
class Lesson {
+str subject
+datetime.time start_time
+datetime.time end_time
+serialize_time(time: datetime.time) str
}
class SingleDaySchedule {
+Literal[1,2,3,4,5,6,7] enable_day
+list~Lesson~ classes
+str name
+Literal['all','odd','even'] weeks
+is_enabled_on_week(int week) bool
+is_enabled_on_day(datetime.date start_day, datetime.date day) bool
}
class Schedule {
+list~SingleDaySchedule~ data
}
CSES --> Schedule : schedules
CSES --> Subject : subjects
Schedule --> SingleDaySchedule
SingleDaySchedule --> Lesson
Lesson --> Subject : subject (by name)
Class diagram for new utility functions and loggerclassDiagram
class utils {
+week_num(datetime.date, datetime.date) int
+ensure_time(str|int|datetime.time) datetime.time
+serialize_time(dumper, datetime.time) yaml.nodes.ScalarNode
+NoAliasDumper
+log
+repr_
}
class NoAliasDumper {
+ignore_aliases(data)
+increase_indent(flow, indentless)
}
utils --> NoAliasDumper
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
大家好 - 我已经审阅了你的更改 - 以下是一些反馈：
- 避免在库内部（utils.py）通过 basicConfig 配置根记录器 — 日志设置应留给导入此模块的应用程序。
__init__.py中对yaml.add_representer和log.info的调用在导入时引入了副作用；考虑将这些推迟到一个显式的初始化函数或类方法中，以防止全局状态污染。- 放弃
WeekType枚举而改用字符串字面量简化了模型，但牺牲了类型安全性和可发现性 — 考虑保留枚举或为允许值添加更严格的验证。
给AI代理的提示
请处理此代码审查中的评论：
## 总体评论
- 避免在库内部（utils.py）通过 basicConfig 配置根记录器 — 日志设置应留给导入此模块的应用程序。
- `__init__.py` 中对 `yaml.add_representer` 和 `log.info` 的调用在导入时引入了副作用；考虑将这些推迟到一个显式的初始化函数或类方法中，以防止全局状态污染。
- 放弃 `WeekType` 枚举而改用字符串字面量简化了模型，但牺牲了类型安全性和可发现性 — 考虑保留枚举或为允许值添加更严格的验证。
## 单个评论
### 评论 1
<location> `cses/__init__.py:139` </location>
<code_context>
+ fp (str): 要写入的文件路径。
+ mode (str, optional): 写入模式，默认值为 ``'w'`` ，即覆盖写入。
+ """
+ os.makedirs(os.path.dirname(fp), exist_ok=True)
+ with open(fp, mode, encoding='utf8') as f:
+ f.write(self.to_yaml())
</code_context>
<issue_to_address>
**issue:** 如果 fp 是一个没有目录的文件名，os.makedirs 可能会失败。
在调用 os.makedirs 之前检查 os.path.dirname(fp) 是否非空，以避免当 fp 只是一个文件名时出现错误。
</issue_to_address>
### 评论 2
<location> `cses/utils.py:12` </location>
<code_context>
+
+import yaml
+
+logging.basicConfig(level=logging.DEBUG,
+ format="[{asctime} - {module}.{funcName}:{lineno}] \t{levelname}:\t {message}",
+ style="{",
</code_context>
<issue_to_address>
**issue (complexity):** 考虑通过将记录器和 repr 设置推迟到运行时且仅在需要时进行，来消除导入时的副作用。
```suggestion
# 1) 移除全局 basicConfig 副作用，并将记录器设置移至应用程序的入口点
# （例如，在 CLI 或主模块的 `if __name__ == "__main__":` 下）。
--- a/your_module.py
@@
- import logging
- from sys import stderr
-
- logging.basicConfig(
- level=logging.DEBUG,
- format="[{asctime} - {module}.{funcName}:{lineno}] \t{levelname}:\t {message}",
- style="{",
- stream=stderr,
- )
- log = logging.getLogger(__name__)
- log.info(f"Loaded logger: {log!r}")
+ import logging
+
+ # library code should not call basicConfig() at import time.
+ log = logging.getLogger(__name__)
```
```suggestion
# 2) 仅在实际需要时才延迟初始化自定义 repr，以避免导入时膨胀。
--- a/your_module.py
@@
- import reprlib
-
- repr_ = reprlib.Repr(
- maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3, maxset=3,
- maxfrozenset=3, maxdeque=3, maxstring=30, maxlong=50, maxother=30,
- fillvalue=' ... ', indent=None
- ).repr
+ import reprlib
+
+ _repr_instance = None
+ def get_repr():
+ global _repr_instance
+ if _repr_instance is None:
+ _repr_instance = reprlib.Repr(
+ maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3,
+ maxset=3, maxfrozenset=3, maxdeque=3, maxstring=30,
+ maxlong=50, maxother=30, fillvalue=' ... ', indent=None
+ ).repr
+ return _repr_instance
```
# 其他地方的使用：
```python
# old:
# log.debug(repr_(some_obj))
# new:
log.debug(get_repr()(some_obj))
```
这些更改消除了导入时的副作用，并将昂贵的设置推迟到实际使用时进行，而不会改变现有行为。
</issue_to_address>
### 评论 3
<location> `cses/utils.py:44-45` </location>
<code_context>
def week_num(start_day: datetime.date, day: datetime.date) -> int:
"""
计算指定日期是从开始日期开始的第多少周。
Args:
day (datetime.date): 要计算周次的日期
start_day (datetime.date): 课程开始的日期，用于计算周次
Returns:
int: 指定日期是从开始日期开始的第多少周
Examples:
>>> week_num(datetime.date(2025, 9, 1), datetime.date(2025, 9, 4))
1
>>> week_num(datetime.date(2025, 9, 1), datetime.date(2025, 9, 16))
3
>>> week_num(datetime.date(2025, 9, 1), datetime.date(2025, 10, 24))
8
"""
res = (day - start_day).days // 7 + 1
return res
</code_context>
<issue_to_address>
**suggestion (code-quality):** 内联立即返回的变量 ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))
```suggestion
return (day - start_day).days // 7 + 1
```
</issue_to_address>帮助我更有用！请点击每个评论的 👍 或 👎，我将利用这些反馈来改进你的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- Avoid configuring the root logger via basicConfig inside the library (utils.py) — logging setup should be left to the application importing this module.
- The calls to yaml.add_representer and log.info in init.py introduce side effects at import time; consider deferring these to an explicit init function or classmethod to prevent global state pollution.
- Dropping the WeekType enum in favor of string literals simplifies the model but sacrifices type safety and discoverability—consider retaining an enum or adding stricter validation for allowed values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid configuring the root logger via basicConfig inside the library (utils.py) — logging setup should be left to the application importing this module.
- The calls to yaml.add_representer and log.info in __init__.py introduce side effects at import time; consider deferring these to an explicit init function or classmethod to prevent global state pollution.
- Dropping the WeekType enum in favor of string literals simplifies the model but sacrifices type safety and discoverability—consider retaining an enum or adding stricter validation for allowed values.
## Individual Comments
### Comment 1
<location> `cses/__init__.py:139` </location>
<code_context>
+ fp (str): 要写入的文件路径。
+ mode (str, optional): 写入模式，默认值为 ``'w'`` ，即覆盖写入。
+ """
+ os.makedirs(os.path.dirname(fp), exist_ok=True)
+ with open(fp, mode, encoding='utf8') as f:
+ f.write(self.to_yaml())
</code_context>
<issue_to_address>
**issue:** os.makedirs may fail if fp is a filename without a directory.
Check if os.path.dirname(fp) is non-empty before calling os.makedirs to avoid errors when fp is just a filename.
</issue_to_address>
### Comment 2
<location> `cses/utils.py:12` </location>
<code_context>
+
+import yaml
+
+logging.basicConfig(level=logging.DEBUG,
+ format="[{asctime} - {module}.{funcName}:{lineno}] \t{levelname}:\t {message}",
+ style="{",
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing import-time side effects by deferring logger and repr setup to runtime and only when needed.
```suggestion
# 1) Remove global basicConfig side‐effect and move logger setup to your application's entry point
# (e.g. under `if __name__ == "__main__":` in your CLI or main module).
--- a/your_module.py
@@
- import logging
- from sys import stderr
-
- logging.basicConfig(
- level=logging.DEBUG,
- format="[{asctime} - {module}.{funcName}:{lineno}] \t{levelname}:\t {message}",
- style="{",
- stream=stderr,
- )
- log = logging.getLogger(__name__)
- log.info(f"Loaded logger: {log!r}")
+ import logging
+
+ # library code should not call basicConfig() at import time.
+ log = logging.getLogger(__name__)
```
```suggestion
# 2) Lazy‐initialize the custom repr only when actually needed to avoid import-time bloat.
--- a/your_module.py
@@
- import reprlib
-
- repr_ = reprlib.Repr(
- maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3, maxset=3,
- maxfrozenset=3, maxdeque=3, maxstring=30, maxlong=50, maxother=30,
- fillvalue=' ... ', indent=None
- ).repr
+ import reprlib
+
+ _repr_instance = None
+ def get_repr():
+ global _repr_instance
+ if _repr_instance is None:
+ _repr_instance = reprlib.Repr(
+ maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3,
+ maxset=3, maxfrozenset=3, maxdeque=3, maxstring=30,
+ maxlong=50, maxother=30, fillvalue=' ... ', indent=None
+ ).repr
+ return _repr_instance
```
# Usage elsewhere:
```python
# old:
# log.debug(repr_(some_obj))
# new:
log.debug(get_repr()(some_obj))
```
These changes remove import‐time side effects and defer expensive setup until actually used, without altering existing behavior.
</issue_to_address>
### Comment 3
<location> `cses/utils.py:44-45` </location>
<code_context>
def week_num(start_day: datetime.date, day: datetime.date) -> int:
"""
计算指定日期是从开始日期开始的第多少周。
Args:
day (datetime.date): 要计算周次的日期
start_day (datetime.date): 课程开始的日期，用于计算周次
Returns:
int: 指定日期是从开始日期开始的第多少周
Examples:
>>> week_num(datetime.date(2025, 9, 1), datetime.date(2025, 9, 4))
1
>>> week_num(datetime.date(2025, 9, 1), datetime.date(2025, 9, 16))
3
>>> week_num(datetime.date(2025, 9, 1), datetime.date(2025, 10, 24))
8
"""
res = (day - start_day).days // 7 + 1
return res
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))
```suggestion
return (day - start_day).days // 7 + 1
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| fp (str): 要写入的文件路径。 | ||
| mode (str, optional): 写入模式，默认值为 ``'w'`` ，即覆盖写入。 | ||
| """ | ||
| os.makedirs(os.path.dirname(fp), exist_ok=True) |
There was a problem hiding this comment.
issue: 如果 fp 是一个没有目录的文件名，os.makedirs 可能会失败。
在调用 os.makedirs 之前检查 os.path.dirname(fp) 是否非空，以避免当 fp 只是一个文件名时出现错误。
Original comment in English
issue: os.makedirs may fail if fp is a filename without a directory.
Check if os.path.dirname(fp) is non-empty before calling os.makedirs to avoid errors when fp is just a filename.
|
|
||
| import yaml | ||
|
|
||
| logging.basicConfig(level=logging.DEBUG, |
There was a problem hiding this comment.
issue (complexity): 考虑通过将记录器和 repr 设置推迟到运行时且仅在需要时进行，来消除导入时的副作用。
Suggested change
| logging.basicConfig(level=logging.DEBUG, | |
| # 1) 移除全局 basicConfig 副作用，并将记录器设置移至应用程序的入口点 | |
| # （例如，在 CLI 或主模块的 `if __name__ == "__main__":` 下）。 | |
| --- a/your_module.py | |
| @@ | |
| - import logging | |
| - from sys import stderr | |
| - | |
| - logging.basicConfig( | |
| - level=logging.DEBUG, | |
| - format="[{asctime} - {module}.{funcName}:{lineno}] \t{levelname}:\t {message}", | |
| - style="{", | |
| - stream=stderr, | |
| - ) | |
| - log = logging.getLogger(__name__) | |
| - log.info(f"Loaded logger: {log!r}") | |
| + import logging | |
| + | |
| + # library code should not call basicConfig() at import time. | |
| + log = logging.getLogger(__name__) |
Suggested change
| logging.basicConfig(level=logging.DEBUG, | |
| # 2) 仅在实际需要时才延迟初始化自定义 repr，以避免导入时膨胀。 | |
| --- a/your_module.py | |
| @@ | |
| - import reprlib | |
| - | |
| - repr_ = reprlib.Repr( | |
| - maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3, maxset=3, | |
| - maxfrozenset=3, maxdeque=3, maxstring=30, maxlong=50, maxother=30, | |
| - fillvalue=' ... ', indent=None | |
| - ).repr | |
| + import reprlib | |
| + | |
| + _repr_instance = None | |
| + def get_repr(): | |
| + global _repr_instance | |
| + if _repr_instance is None: | |
| + _repr_instance = reprlib.Repr( | |
| + maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3, | |
| + maxset=3, maxfrozenset=3, maxdeque=3, maxstring=30, | |
| + maxlong=50, maxother=30, fillvalue=' ... ', indent=None | |
| + ).repr | |
| + return _repr_instance |
其他地方的使用：
# old:
# log.debug(repr_(some_obj))
# new:
log.debug(get_repr()(some_obj))这些更改消除了导入时的副作用，并将昂贵的设置推迟到实际使用时进行，而不会改变现有行为。
Original comment in English
issue (complexity): Consider removing import-time side effects by deferring logger and repr setup to runtime and only when needed.
Suggested change
| logging.basicConfig(level=logging.DEBUG, | |
| # 1) Remove global basicConfig side‐effect and move logger setup to your application's entry point | |
| # (e.g. under `if __name__ == "__main__":` in your CLI or main module). | |
| --- a/your_module.py | |
| @@ | |
| - import logging | |
| - from sys import stderr | |
| - | |
| - logging.basicConfig( | |
| - level=logging.DEBUG, | |
| - format="[{asctime} - {module}.{funcName}:{lineno}] \t{levelname}:\t {message}", | |
| - style="{", | |
| - stream=stderr, | |
| - ) | |
| - log = logging.getLogger(__name__) | |
| - log.info(f"Loaded logger: {log!r}") | |
| + import logging | |
| + | |
| + # library code should not call basicConfig() at import time. | |
| + log = logging.getLogger(__name__) |
Suggested change
| logging.basicConfig(level=logging.DEBUG, | |
| # 2) Lazy‐initialize the custom repr only when actually needed to avoid import-time bloat. | |
| --- a/your_module.py | |
| @@ | |
| - import reprlib | |
| - | |
| - repr_ = reprlib.Repr( | |
| - maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3, maxset=3, | |
| - maxfrozenset=3, maxdeque=3, maxstring=30, maxlong=50, maxother=30, | |
| - fillvalue=' ... ', indent=None | |
| - ).repr | |
| + import reprlib | |
| + | |
| + _repr_instance = None | |
| + def get_repr(): | |
| + global _repr_instance | |
| + if _repr_instance is None: | |
| + _repr_instance = reprlib.Repr( | |
| + maxlevel=3, maxtuple=3, maxlist=3, maxarray=3, maxdict=3, | |
| + maxset=3, maxfrozenset=3, maxdeque=3, maxstring=30, | |
| + maxlong=50, maxother=30, fillvalue=' ... ', indent=None | |
| + ).repr | |
| + return _repr_instance |
Usage elsewhere:
# old:
# log.debug(repr_(some_obj))
# new:
log.debug(get_repr()(some_obj))These changes remove import‐time side effects and defer expensive setup until actually used, without altering existing behavior.
Comment on lines
+44
to
+45
| res = (day - start_day).days // 7 + 1 | ||
| return res |
There was a problem hiding this comment.
suggestion (code-quality): 内联立即返回的变量 (inline-immediately-returned-variable)
Suggested change
| res = (day - start_day).days // 7 + 1 | |
| return res | |
| return (day - start_day).days // 7 + 1 |
Original comment in English
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
Suggested change
| res = (day - start_day).days // 7 + 1 | |
| return res | |
| return (day - start_day).days // 7 + 1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Sourcery 总结
重构并增强 CSES 库的 API，以提高可用性、序列化和类型验证能力
新功能：
改进：
文档：
杂项：
Original summary in English
Summary by Sourcery
Refactor and enhance the CSES library API to improve usability, serialization, and type validation
New Features:
Enhancements:
Documentation:
Chores: