v1.17.0 breaking Annoy4S tests

[mridang]

Hi all. I'm trying to upgrade https://github.com/annoy4s/annoy4s to use the latest v1.17.0 release but one of the tests seem to have changed.

The following test has been cannibalized from https://github.com/annoy4s/annoy4s/blob/0.9.0/src/test/scala/annoy4s/AnnoySpec.scala#L172-L187 and reimplemented in Python.

from common import TestCase
from common import TestCase

class Annoy4SCompatTest(TestCase):
    def test_get_nns_by_vector(self):
        #https://raw.githubusercontent.com/annoy4s/annoy4s/0.9.0/src/test/resources/searchk-test-vector
        with open("searchk-test-vector") as f:
            content = f.readlines()
        content = [line.rstrip().split(" ") for line in content]

        index = AnnoyIndex(10, metric='angular')
        for line in content:
            vector = [float(vector) for vector in line[1:]]
            index.add_item(int(line[0]), vector)
        index.build(2)

        self.assertEqual(index.get_nns_by_item(1, 10, search_k=2, include_distances=False), [1, 54, 55, 60, 76, 8, 32, 33])
        self.assertEqual(index.get_nns_by_item(1, 10, search_k=-1, include_distances=False), [1, 69, 39, 87, 54, 29, 62, 55, 21, 35])

This test when run as part of the default test suite seems to pass on all versions up v1.16.3 but then fails on v1.17.0. Is this a bug or is the test simply incorrect?

~/zooma/annoy   abba0fbe v1.17.0 ?  python setup.py nosetests --tests=test/annoy4s_compat_test.py                                                                                                ✔  .env  10778  14:46:55 04/04/2021
running nosetests
running egg_info
writing annoy.egg-info/PKG-INFO
writing dependency_links to annoy.egg-info/dependency_links.txt
writing top-level names to annoy.egg-info/top_level.txt
reading manifest file 'annoy.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'annoy.egg-info/SOURCES.txt'
building 'annoy.annoylib' extension
clang -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include -I/Users/mridang/zooma/annoy/.env/include -I/Users/mridang/.pyenv/versions/3.6.3/include/python3.6m -c src/annoymodule.cc -o build/temp.macosx-10.14-x86_64-3.6/src/annoymodule.o -D_CRT_SECURE_NO_WARNINGS -march=native -O3 -ffast-math -fno-associative-math -DANNOYLIB_MULTITHREADED_BUILD -std=c++14 -mmacosx-version-min=10.12
clang++ -bundle -undefined dynamic_lookup -L/usr/local/opt/readline/lib -L/usr/local/opt/readline/lib -L/usr/local/opt/openssl@1.1/lib -L/Users/mridang/.pyenv/versions/3.6.3/lib -L/usr/local/opt/readline/lib -L/usr/local/opt/readline/lib -L/usr/local/opt/openssl@1.1/lib -L/Users/mridang/.pyenv/versions/3.6.3/lib build/temp.macosx-10.14-x86_64-3.6/src/annoymodule.o -o build/lib.macosx-10.14-x86_64-3.6/annoy/annoylib.cpython-36m-darwin.so -stdlib=libc++ -mmacosx-version-min=10.12
copying build/lib.macosx-10.14-x86_64-3.6/annoy/annoylib.cpython-36m-darwin.so -> annoy
F
======================================================================
FAIL: test_get_nns_by_vector (annoy4s_compat_test.Annoy4SCompatTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/mridang/zooma/annoy/test/annoy4s_compat_test.py", line 18, in test_get_nns_by_vector
    self.assertEqual(index.get_nns_by_item(1, 10, search_k=2, include_distances=False), [1, 54, 55, 60, 76, 8, 32, 33])
AssertionError: Lists differ: [1, 69, 39, 87, 62, 97, 31, 96, 99] != [1, 54, 55, 60, 76, 8, 32, 33]

First differing element 1:
69
54

First list contains 1 additional elements.
First extra element 8:
99

- [1, 69, 39, 87, 62, 97, 31, 96, 99]
+ [1, 54, 55, 60, 76, 8, 32, 33]

----------------------------------------------------------------------
Ran 1 test in 0.003s

FAILED (failures=1)

[mridang]

@erikbern would you be sanity check this please? Thank you. 🙏🏽

[erikbern]

This test seems a bit weird – search_k=2 is so low it's basically pointless to test it. I would just remove that assertion if that's an easy thing to do. search_k should be some large number such as 1000 or more... it governs (roughly) how many vectors it will consider before terminating the search.