Skip to content

Update: move const to types make code simpler #204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vcaesar merged 1 commit into from
Mar 12, 2026
Merged

Update: move const to types make code simpler #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions consts/dict_file.go
View file
Open in desktop

This file was deleted.

5 changes: 3 additions & 2 deletions dict_1.16.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright 2016 The go-ego Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// Copyright 2016 The go-ego Project Developers.
//
// See the COPYRIGHT file at the top-level directory of this distribution and at
// https://github.com/go-ego/gse/blob/master/LICENSE
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
Expand Down
3 changes: 1 addition & 2 deletions dict_1.16_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
_ "embed"
"testing"

"github.com/go-ego/gse/consts"
"github.com/go-ego/gse/types"
"github.com/vcaesar/tt"
)
Expand Down Expand Up @@ -100,7 +99,7 @@ func TestLoadTFIDFDictStr(t *testing.T) {
a := []*types.LoadDictFile{}
a = append(a, &types.LoadDictFile{
FilePath: "/workspaces/gse/data/dict/zh/tf_idf.txt",
FileType: consts.LoadDictTypeTFIDF,
FileType: types.LoadDictTypeTFIDF,
})
seg.LoadTFIDFDict(a)
}
5 changes: 2 additions & 3 deletions hmm/extracker/tag_extracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"sort"

"github.com/go-ego/gse"
"github.com/go-ego/gse/consts"
"github.com/go-ego/gse/hmm/relevance"
"github.com/go-ego/gse/hmm/segment"
"github.com/go-ego/gse/types"
Expand Down Expand Up @@ -86,10 +85,10 @@ func (t *TagExtracter) LoadBM25(setting *types.BM25Setting, fileList []*types.Lo
for _, v := range fileList {
switch v.FileType {

case consts.LoadDictCorpus:
case types.LoadDictCorpus:
corpusBM25 = append(corpusBM25, v.FilePath)

case consts.LoadDictTypeBM25:
case types.LoadDictTypeBM25:
dictBM25 = append(dictBM25, v.FilePath)
}
}
Expand Down
13 changes: 6 additions & 7 deletions hmm/relevance/bm25.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"unicode/utf8"

"github.com/go-ego/gse"
"github.com/go-ego/gse/consts"
"github.com/go-ego/gse/hmm/segment"
"github.com/go-ego/gse/hmm/stopwords"
"github.com/go-ego/gse/types"
Expand Down Expand Up @@ -74,7 +73,7 @@ func (bm25 *BM25) LoadDict(files ...string) error {
for i, v := range files {
dictFiles[i] = &types.LoadDictFile{
FilePath: v,
FileType: consts.LoadDictTypeBM25,
FileType: types.LoadDictTypeBM25,
}
}

Expand All @@ -85,7 +84,7 @@ func (bm25 *BM25) LoadDict(files ...string) error {
func (bm25 *BM25) LoadDictStr(dictStr string) error {
dictFile := &types.LoadDictFile{
FilePath: dictStr,
FileType: consts.LoadDictTypeBM25,
FileType: types.LoadDictTypeBM25,
}
return bm25.Seg.LoadTFIDFDictStr(dictFile)
}
Expand Down Expand Up @@ -184,15 +183,15 @@ func NewBM25(bm25Setting *types.BM25Setting) Relevance {
// init value
if bm25Setting == nil {
bm25Setting = &types.BM25Setting{
K1: consts.BM25DefaultK1,
B: consts.BM25DefaultB,
K1: types.BM25DefaultK1,
B: types.BM25DefaultB,
}
}
if bm25Setting.K1 == 0 {
bm25Setting.K1 = consts.BM25DefaultK1
bm25Setting.K1 = types.BM25DefaultK1
}
if bm25Setting.B == 0 {
bm25Setting.K1 = consts.BM25DefaultB
bm25Setting.K1 = types.BM25DefaultB

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After fixing the defaulting logic for B, it would be good to add a unit test covering NewBM25 defaults (e.g., nil settings and settings with B == 0) to prevent regressions. There are already Go tests in this repo (e.g. dict_1.16_test.go), but BM25 defaulting isn’t currently exercised.

Suggested change
bm25Setting.K1 = types.BM25DefaultB
bm25Setting.B = types.BM25DefaultB

Copilot uses AI. Check for mistakes.

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In NewBM25, when bm25Setting.B == 0 the code assigns BM25DefaultB to bm25Setting.K1 instead of bm25Setting.B. This leaves B at 0 and changes K1 unexpectedly, which will produce incorrect BM25 weights. Set the default on the B field here.

Suggested change
bm25Setting.K1 = types.BM25DefaultB
bm25Setting.B = types.BM25DefaultB

Copilot uses AI. Check for mistakes.
}

bm25 := &BM25{
Expand Down
5 changes: 2 additions & 3 deletions hmm/relevance/tfidf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"unicode/utf8"

"github.com/go-ego/gse"
"github.com/go-ego/gse/consts"
"github.com/go-ego/gse/hmm/segment"
"github.com/go-ego/gse/hmm/stopwords"
"github.com/go-ego/gse/types"
Expand Down Expand Up @@ -59,7 +58,7 @@ func (t *TFIDF) LoadDict(files ...string) error {
for i, v := range files {
dictFiles[i] = &types.LoadDictFile{
FilePath: v,
FileType: consts.LoadDictTypeTFIDF,
FileType: types.LoadDictTypeTFIDF,
}
}

Expand All @@ -70,7 +69,7 @@ func (t *TFIDF) LoadDict(files ...string) error {
func (t *TFIDF) LoadDictStr(dictStr string) error {
dictFile := &types.LoadDictFile{
FilePath: dictStr,
FileType: consts.LoadDictTypeTFIDF,
FileType: types.LoadDictTypeTFIDF,
}
return t.Seg.LoadTFIDFDictStr(dictFile)
}
Expand Down
27 changes: 27 additions & 0 deletions types/dict_file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,30 @@ type BM25Setting struct {
// and we define B = 0.75 for defult value in `consts/dict_file.go`
B float64
}

const (
// dict file type to loading

// LoadDictTypeIDF dict of IDF to loading
LoadDictTypeIDF = iota + 1
Comment on lines +23 to +27

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the constants live in types/dict_file.go, several existing field comments in this file still point to consts/dict_file.go for where defaults/types are defined. Please update those references so the documentation matches the new location (and avoids pointing to a deleted package/file).

Copilot uses AI. Check for mistakes.

// LoadDictTypeTFIDF dict of TFIDF to loading
LoadDictTypeTFIDF
Comment on lines +23 to +30

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change removes the consts package and moves these exported constants into types. Since the module path is still github.com/go-ego/gse (no v2+ major version), dropping github.com/go-ego/gse/consts is a breaking API change for downstream users. Consider keeping a small consts package that re-exports these constants (possibly with deprecation comments) to preserve backward compatibility.

Copilot uses AI. Check for mistakes.

// LoadDictTypeBM25 dict of BM25 to loading
LoadDictTypeBM25

// LoadDictTypeWithPos dict of with position to loading
LoadDictTypeWithPos

// LoadDictCorpus dict of corpus to loading
LoadDictCorpus
)

const (
// BM25DefaultK1 default k1 value for calculate bm25
BM25DefaultK1 = 1.25

// BM25DefaultK1 default B value for calculate bm25

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment for BM25DefaultB says BM25DefaultK1 default B value..., which is inaccurate/misleading. Please rename the comment to refer to BM25DefaultB (and ideally describe it as the default B parameter for BM25).

Suggested change
// BM25DefaultK1 default B value for calculate bm25
// BM25DefaultB default B parameter value for calculating bm25

Copilot uses AI. Check for mistakes.
BM25DefaultB = 0.75
)
Loading