Skip to content

Conversation

@dkobak
Copy link
Contributor

@dkobak dkobak commented Nov 3, 2025

Trying to fix #210 as discussed there.

See https://docs.python.org/3.12/library/tempfile.html#examples for an official example of how to work with NamedTemporaryFile when one needs to write smth into it and then read from it:

# create a temporary file using a context manager
# close the file, use the name to open the file again

with tempfile.NamedTemporaryFile(delete_on_close=False) as fp:
    fp.write(b'Hello world!')
    fp.close()
    # the file is closed, but not removed
    
    # open the file again by using its name
    with open(fp.name, mode='rb') as f:
        f.read()
# file is now removed

@dkobak dkobak changed the title Switch to tempfile.TemporaryFile Switch to tempfile.NamedTemporaryFile Nov 3, 2025
@dkobak
Copy link
Contributor Author

dkobak commented Nov 4, 2025

The current state of this PR is that the tests pass on Linux and OSX (for Python >= 3.12) but fail on Windows. Curiously, only one single test fails:

tests/test_nearest_neighbors.py::TestAnnoy::test_pickle_with_built_index FAILED [ 41%]
tests/test_nearest_neighbors.py::TestAnnoy::test_pickle_without_built_index PASSED [ 41%]
tests/test_nearest_neighbors.py::TestAnnoy::test_pickle_without_built_index_cleans_up_fname PASSED [ 42%]
tests/test_nearest_neighbors.py::TestHNSW::test_pickle_with_built_index PASSED  [ 52%]
tests/test_nearest_neighbors.py::TestHNSW::test_pickle_without_built_index PASSED [ 53%]
tests/test_nearest_neighbors.py::TestHNSW::test_pickle_without_built_index_cleans_up_fname PASSED [ 54%]

Note that test_pickle_with_built_index passes for HNSW but fails for Annoy! Even though the tests are identical, and __getstate__() is implemented the same way for HNSW and Annoy.

@pavlin-policar
Copy link
Owner

So, as we discussed, the state of this PR is as follows:

  • this makes a step in the right direction. It fixes the base problem, but some tests still fail for some reason
  • the fix includes a Python >=3.12 feature while openTSNE currently still supports 3.10. The end-of-life of Python 3.11 is sometime at the end of 2027, so this is not an entirely viable fix until then. We can wrap in an if statement or something, but it's not clean

So, from my understanding, this is a step in the right direction, but not something that can be merged.

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.

[Windows] saving/loading TSNEEmbedding objects to pickle, Directory error

2 participants