GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix setup-dev.py for local imports that override stdlib (e.g. on Python 3.10) by krfricke · Pull Request #31829 · ray-project/ray

Conversation

@krfricke

@krfricke krfricke commented Jan 21, 2023

Copy link
Copy Markdown
Contributor

Signed-off-by: Kai Fricke [email protected]

Why are these changes needed?

Python adds the current working directory as the first entry to sys.path . In some python versions, this will lead stdlib libraries to import modules in the working directory instead of other stdlib modules (python/cpython#101210).

In our case, the conflict is types.py. Because types.py contains a PublicAPI we don't want to just rename it without proper deprecation. Instead we fix the sys.path in setup-dev.py as this is the only location where the problem predictably comes up.

Related issue number

Closes #31458

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Kai Fricke
Fix setup-dev.py on Python 3.10
da2d0e2 
Signed-off-by: Kai Fricke <[email protected]>
@krfricke krfricke changed the title Fix setup-dev.py on Python 3.10 Fix setup-dev.py for local imports that override stdlib (e.g. on Python 3.10) Jan 21, 2023
@krfricke
krfricke requested review from Yard1 and pcmoritz January 21, 2023 02:06
@krfricke krfricke assigned pcmoritz and Yard1 Jan 21, 2023
@krfricke krfricke mentioned this pull request Jan 21, 2023
Error when building from source: the existence of python/ray/types.py causes an ImportError #31458
Closed
@krfricke
krfricke requested a review from sven1977 January 21, 2023 02:10
@krfricke krfricke assigned sven1977 Jan 21, 2023
sven1977
sven1977 reviewed Jan 24, 2023
View reviewed changes
Comment thread python/ray/setup-dev.py
# To avoid import errors, we move the current working dir to the end of sys.path.
this_dir = os.path.dirname(__file__)
if this_dir in sys.path:
cur = sys.path.remove(this_dir)

@sven1977 sven1977 Jan 24, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Nit: Do we need the cur here? If not, should we drop assigning this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sven1977
sven1977 approved these changes Jan 24, 2023
View reviewed changes

@sven1977 sven1977 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix @krfricke !

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@krfricke
krfricke merged commit 3e1cdeb into ray-project:master Jan 24, 2023
@krfricke
krfricke deleted the setup-dev-path branch January 24, 2023 17:04
This was referenced Aug 26, 2023
updates to setup-dev.py to work around the types.py import issues #38948
Closed
[installation] Ray Installation broken with circular import on virtualenvs #29587
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sven1977 sven1977 sven1977 approved these changes
@pcmoritz pcmoritz Awaiting requested review from pcmoritz
@Yard1 Yard1 Awaiting requested review from Yard1

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@pcmoritz pcmoritz

@sven1977 sven1977

@Yard1 Yard1

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error when building from source: the existence of python/ray/types.py causes an ImportError

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@krfricke @sven1977 @pcmoritz @Yard1
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.