Skip to content

Make corpus_score_args checks stricter - #294

Merged
martinpopel merged 2 commits into
mjpost:masterfrom
sentiasm:check-corpus-score-args
Jul 14, 2026
Merged

martinpopel merged 2 commits into
mjpost:masterfrom
sentiasm:check-corpus-score-args

Conversation

@sentiasm

@sentiasm sentiasm commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

I did this to catch an easy error to make when you give refs as a sequence of strings, not a sequence of sequence of strings. If you give a sequence of strings, it still gets accepted but I think the scores get calculated wrong.

@martinpopel

Copy link
Copy Markdown
Collaborator

Good idea. Thank you. Can you please add tests (that check that "when you give refs as a sequence of strings, not a sequence of sequence of strings" you'll get this error).

@sentiasm

Copy link
Copy Markdown
Contributor Author

I added some tests and expanded the scope to include sentence_score_args too. Let me know what you need. Thanks.

Add tests
Add changelog entry
@martinpopel

Copy link
Copy Markdown
Collaborator

The tests on Windows with Python 3.12 fail with ./test.sh: line 69: CMD: unbound variable.

Does anyone (@mjpost, @ozancaglayan) know how to fix it? I don't have time for debugging it (and no Windows box for testing). Also, this failure seems to be unrelated to this PR.
So if there are no objections I will merge it anyway (perhaps tomorrow).

@martinpopel
martinpopel merged commit e3e887e into mjpost:master Jul 14, 2026
0 of 15 checks passed
@mjpost

mjpost commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Sorry @martinpopel, just saw this. I don't have a way to test locally. We could add Windows machines to the Github testing. Thanks for merging.

@martinpopel

Copy link
Copy Markdown
Collaborator

We could add Windows machines to the Github testing.

Windows machines are already covered by our GitHub action tests. That's the place where I see the ./test.sh: line 69: CMD: unbound variable. error.

Luckily, it seems this error is deterministic (within three tests done so far, all three failed in Windows with Python 3.12 with the same error), so I've prompted GitHub Copilot to fix this bug (I'm on vacation with a small screen and no energy for delving into Windows-specific bugs), so let's wait for the result.

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.

3 participants