Skip to content

Fix missing readline error on Windows #503

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.

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
merged 6 commits into from
May 28, 2025
Merged

Fix missing readline error on Windows #503

merged 6 commits into from
May 28, 2025

Conversation

jhamon
Copy link
Collaborator

@jhamon jhamon commented May 27, 2025

Problem

Missing readline dependency error when using list_models function. Reported in #502

Solution

This error was the result of the Cursor assistant putting some code into pinecone/scripts/repl.py that should only have gone into scripts/repl.py (and not been part of the package distribution).

This wasn't caught in CI because:

  • readline is included in our dev dependencies, which are installed in CI for testing purposes. readline is part of the python standard library, so even though this reference was inadvertantly referenced, the presence of these import statements should not cause a problem on dev machines (mac) or CI (linux). However, readline is not available for Windows which is why this error was occurring for the person who reported.
  • Currently we do not have any Windows-specific testing in CI which is why this error shipped unnoticed. So I implemented some new tests to check the package can be installed on Windows in this PR.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Test Plan

  • Going to make a dev build and verify list_models() works in google colab (where dev dependencies are not present) (7.0.2.dev1)
  • Tested in Colab with 7.0.0 and 7.0.1 and could not reproduce the problem; after further research it turns out this is a windows specific problem. I should probably add some windows builds into the test matrix.
  • Added new installation tests workflow to verify Windows install

@jhamon jhamon marked this pull request as ready for review May 27, 2025 13:49
Copy link
Contributor

@rohanshah18 rohanshah18 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jhamon jhamon changed the title Fix errors related to missing readline dependency Fix missing readline error on Windows May 27, 2025
@jhamon jhamon force-pushed the jhamon/missing-readline branch from 17ea897 to bdaf7a9 Compare May 27, 2025 15:52
uses: './.github/workflows/testing-integration.yaml'
secrets: inherit
needs: unit-tests
# integration-tests:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these commented blocks need to come out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, thanks for flagging. I just put them back. I disabled them temporarily to iterate quicker on some installation tests I just added.

@jhamon jhamon merged commit fa5f3e7 into main May 28, 2025
90 checks passed
@jhamon jhamon deleted the jhamon/missing-readline branch May 28, 2025 15:04
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