Conversation
|
|
There was a problem hiding this comment.
Pull request overview
This PR updates dictionary initialization logic in the segmenter to rely on seg.Dict == nil rather than a separate seg.Load flag, bumps the library version constant, and removes an unused test.
Changes:
- Bump
Versionconstant ingse.go. - Update
LoadDict/LoadTFIDFDictinitialization guard to checkseg.Dict == nil(and stop usingseg.Load). - Remove
dict_util_test.go(test that only printed values and performed no assertions).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| gse.go | Updates the exported version string. |
| dict_util.go | Changes dictionary initialization condition from !seg.Load to seg.Dict == nil and removes some stray whitespace. |
| dict_util_test.go | Removes a non-asserting/printing-only test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if seg.Dict == nil { | ||
| seg.Dict = NewDict() | ||
| seg.Load = true | ||
| // seg.Load = true | ||
| seg.Init() | ||
| } |
There was a problem hiding this comment.
Segmenter still exposes the Load bool field (see segmenter.go), but LoadDict no longer uses it to decide whether to (re)initialize. This silently changes behavior for any callers that relied on seg.Load = false to force a fresh dictionary/init (and it leaves Load effectively unused). Either remove/deprecate the Load field and update callers/tests accordingly, or keep backward-compatible behavior by honoring !seg.Load here (and keeping Load in sync when initializing/emptying).
| if seg.Dict == nil { | ||
| seg.Dict = NewDict() | ||
| seg.Load = true | ||
| // seg.Load = true | ||
| seg.Init() | ||
| } |
There was a problem hiding this comment.
The commented-out // seg.Load = true suggests Load state is still expected to exist, but LoadTFIDFDict no longer updates or checks it. Please either remove this dead/commented code and the Load field entirely, or restore consistent state management (honor !seg.Load and set it true after init) to avoid confusing/partially-removed API behavior.
Please provide Issues links to:
Provide test code:
Description
...