Skip to content
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

bazel_external: Issue from drake dir being on the Python path, and drake/common/__init__.py being auto-created #13320

Closed
EricCousineau-TRI opened this issue May 14, 2020 · 5 comments
Assignees
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters priority: low

Comments

@EricCousineau-TRI
Copy link
Contributor

EricCousineau-TRI commented May 14, 2020

This is the more specific version of this comment: #7871 (comment)

From @pvarin's StackOverflow post:
https://stackoverflow.com/q/61802860/7829525

To reproduce, see my updated example:
drake_bazel_external/apps/bar.py (branch)

Issue 1: Repositories are slapped onto PYTHONPATH.
Issue 2: Autocreated __init__.py

When running, this is the output that I see: (with <-- annotations added)

$ bazel build //apps:bar
$ alias bash-isolate='env -i HOME=$HOME DISPLAY=$DISPLAY SHELL=$SHELL TERM=$TERM USER=$USER PATH=/usr/local/bin:/usr/bin:/bin bash --norc'
$ bash-isolate -c bazel-bin/apps/bar

path:
  {source_tree}/apps
  {runfiles}
  {runfiles}/drake/bindings
  {runfiles}/lcmtypes_bot2_core/lcmtypes
  {runfiles}/lcmtypes_bot2_core
  {runfiles}/lcmtypes_robotlocomotion/lcmtypes
  {runfiles}/lcmtypes_robotlocomotion
  {runfiles}/meshcat_python/src
  {runfiles}/spdlog
  {runfiles}/meshcat_python
  {runfiles}/lcm
  {runfiles}/ignition_math
  {runfiles}/drake                            <-- Issue 1: Repositories are slapped onto PYTHONPATH.
  {runfiles}/drake_external_examples
  /usr/lib/python36.zip
  /usr/lib/python3.6
  /usr/lib/python3.6/lib-dynload
  /usr/lib/python3/dist-packages

common: {runfiles}/drake/common/__init__.py   <-- Issue 2: Autocreated __init__.py

Traceback (most recent call last):
  File "{runfiles}/drake_external_examples/apps/bar.py", line 24, in <module>
    from common import foo
ImportError: cannot import name 'foo'

Note that common should point to drake_external_examples/common/__init__.py, not drake/common/__init__.py.

FTR The overall correct decision is to import using the workspace name. Changing:

from common import foo

to

from drake_external_examples.common import foo

FYI @jwnimmer-tri

@EricCousineau-TRI EricCousineau-TRI changed the title bazel_external: drake is on the Python path, and common/__init__.py is auto-created bazel_external: drake is on the Python path, and drake/common/__init__.py is auto-created May 14, 2020
@EricCousineau-TRI EricCousineau-TRI added component: build system Bazel, CMake, dependencies, memory checkers, linters priority: low labels May 14, 2020
@EricCousineau-TRI EricCousineau-TRI changed the title bazel_external: drake is on the Python path, and drake/common/__init__.py is auto-created bazel_external: Issue from drake being on the Python path,and drake/common/__init__.py being auto-created May 14, 2020
@EricCousineau-TRI EricCousineau-TRI changed the title bazel_external: Issue from drake being on the Python path,and drake/common/__init__.py being auto-created bazel_external: Issue from drake dir being on the Python path, and drake/common/__init__.py being auto-created May 14, 2020
@jwnimmer-tri
Copy link
Collaborator

jwnimmer-tri commented May 14, 2020

With the "Issue 1, 2" callouts above, I can understand what's happening and why it causes friction.

I think legacy_create_init mentions before were confusing me. I don't see how that's relevant here. For example, it would be easy to depend on something in drake that brought in even a real __init__.py that we wrote for ourselves. In general from drake.stuff import other_stuff should work.

It seems to me the only problem is Issue 1 biting us. To mitigate, we either need to (a) change how bazel python sets up the import path project-wide, or (b) show users how to correctly spell imports in the bazel world.

Since (a) seems like a project wide, rcfile-type setting, it's not fixable in Drake anyway. Maybe a prototypical bazelrc in drake-external-examples.

However, doing (b) should be easy and sufficient to resolve the question. I think the only action here is to add a py_library to drake-external-examples that shows how to import your own python code and pydrake code at the same time?

@EricCousineau-TRI
Copy link
Contributor Author

I think the only action here is to add a py_library to drake-external-examples that shows how to import your own python code and pydrake code at the same time?

SGTM. Will submit strawman example.

@EricCousineau-TRI
Copy link
Contributor Author

@jwnimmer-tri
Copy link
Collaborator

Eric is working on the PR for this, so changed assignment.

@jwnimmer-tri jwnimmer-tri added this to To Do in #dynamics-dev Nov 11, 2021
@jwnimmer-tri jwnimmer-tri removed this from To Do - important in #dynamics-dev Jan 10, 2022
@jwnimmer-tri
Copy link
Collaborator

The PR RobotLocomotion/drake-external-examples#175 died on the vine. Closing this as "won't fix". Any anyone is interested, they can work on resurrecting that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters priority: low
Projects
None yet
Development

No branches or pull requests

4 participants