https://pantsbuild.org/ logo
#general
Title
# general
r

rapid-exabyte-76685

03/29/2022, 9:22 AM
Is there any further guidance on how to deal with these errors…
Copy code
[WARN] The target src/xyz.py imports `Abc`, but Pants cannot safely infer a dependency because more than one target owns this module, so it is ambiguous which to use: ['src/abc/__init__.py', 'test/abc/__init__.py'].
test/abc/__init__.py
is blank. The source for
Abc
is in
src/abc/__init__.py
.
src/xyz.py
contains
from abc import Abc
My repo has
src
and
test
roots that largely mirror each other in their hierarchy. Is this a bad pattern? Is there a way to default the dependency inference to consider the
src
root over the
test
root - unless there is an explicit override? Also is there a way to retest the whole codebase for this class of error as it appears to appear for one invocation of
./pants dependencies src/xyz.py
but then disappears (and I think I have other similar issues that I would like to resolve)
h

happy-kitchen-89482

03/29/2022, 1:40 PM
I think the easiest way to solve this is to not have
test
be a source root. Source roots are for resolving imports to source files, and it sounds like you never want to import from any files under
test
? How are your source roots configured right now?
r

rapid-exabyte-76685

03/29/2022, 1:48 PM
Oh ok, that's interesting. I was under the impression that all code needed to be under a source root, regardless of whether it is production or test-only code... e.g.
Copy code
[source]
root_patterns = [
  '/src/python',
  '/test/python',
]
from https://www.pantsbuild.org/docs/source-roots#configuring-source-roots-using-patterns or in my case
Copy code
root_patterns = ["src", "test"]
But yeah, files under
src
would never import from
test
but files under
test
would import from
src
e.g.
test/app/test_app.py
can
from app.something import TheThing
from
src/app/something.py
containing
TheThing
and my test code does appear to need to be told its dependencies to other test code (all under
test
) via
dependencies
on
python_test
to
python_test_utils
targets
h

happy-kitchen-89482

03/29/2022, 1:54 PM
All code does need to be under a source root, but you can set
root_patterns = ["src", "/"]
so that the test code's source root is the repo root, and if you did want to import from it you'd
import <http://test.app|test.app>
That solves the collision
Oh, so you have test code that imports from other code under
test
?
r

rapid-exabyte-76685

03/29/2022, 1:57 PM
yep, shared test classes, etc.
So quite a few
python_test_utils
have been defined
h

happy-kitchen-89482

03/29/2022, 1:57 PM
How does that work today?
How are they importing?
I'm surprised those imports work at the Python level, due to the naming collision with
src/
h

hundreds-father-404

03/29/2022, 1:59 PM
re how to error in this case, you can use this thanks to @bitter-ability-32190: https://www.pantsbuild.org/docs/reference-python-infer#section-unowned-dependency-behavior
r

rapid-exabyte-76685

03/29/2022, 2:04 PM
So the existing tests have some
conftest.py
files, but also have other files like
util.py
or
application_test_harness.py
. So to get to the point where the tests are passing I've defined quite a few
python_test_utils
targets in BUILD files across the
test
side of the source. And the
python_tests
targets generally need to be explicitly told to depend on these
python_test_utils
... but that might be clearer once I understand the
infer
side of things
h

hundreds-father-404

03/29/2022, 2:05 PM
they need to be explicitly declared specifically because of warnings like this of ambiguity? It should be rare you need to explicitly set dependencies, so we definitely want to figure that out
Ack that this is somewhat disruptive, but one option is to update your imports to use a new source root so that there is no ambiguity. For example, if you take Benjy's suggestion to use
/
rather than
test
as a source root, then you'd update imports to be
<http://test.my|test.my>_project.foo
rather than
my_project.foo
r

rapid-exabyte-76685

03/29/2022, 2:07 PM
there's definitely the ambiguity side of things, but even then it feels like I have more going on in BUILD files than there should be? alas, I need to sign off for the moment. just a question on that python-infer documentation...
Backend:
pants.backend.awslambda.python
Is that correct, more a general python thing than awslamba?
h

happy-kitchen-89482

03/29/2022, 2:17 PM
Backend: pants.backend.awslambda.python
Oooh, you've found a bug in the code that generates this documentation...
cc @curved-television-6568
So, my question is, ignoring Pants, how those test-imports-from-test work at the Python level
Purely as a Python thing, since you're not using "namespace packages", I would have expected
import abc.test_util.TestUtilThing
to fail because presumably your PYTHONPATH has
src/
before
test/
, so Python will look for
abc
in
src
first, find it there and then fail to find
test_util
under it. (If your PYTHONPATH has
test/
before
src/
then your tests should fail to import anything from
src/
).
So I'm a little surprised that this has ever worked, outside of Pants.
Figuring out how that works on the Python side will inform how to teach Pants to let it keep doing that, maybe
1
r

rapid-exabyte-76685

03/29/2022, 11:06 PM
AH, I have made a rookie error 🤦 Some context, my repo currently builds a service that can currently act in two different modes, which mode it is operating in is dictated by a command line argument. There is already some internal modularisation for each mode. So we want to turn this into a proper monorepo so each mode is a distinct separate service with a shared core, for stronger guarantees that each deployed service only contains the code that it requires to perform its function - we are adding more modes and the individual modes may become more complex over time. Pants seems ideal to support this. The structure of the repo before I started my implementation of Pants was (names have been changed to protect the innocent 😅)...
Copy code
/app
  cli.py # this being the module that would be run from the command line with an argument to indicate the mode and
         # which class to invoke in /app/modes/mode1, /app/modes/mode2, etc.
  /modes
    /mode1 ...code for mode1, imports from shared1, shared2 as appropriate
    /mode2
    ... # more modes to come.
  /shared1
  /shared2
/test
  /app
    /modes
       /mode1 - tests for /app/modes/mode1
       ...etc...
To spike 1. my implementation of Pants AND 2. refactoring to a monorepo (only worked on 1 so far) I have copied everything from
/app
under
/src
- while leaving
/app
in place so that the 'old way' still works (although CI for this branch is bit of a mess at the moment TBH 😅)
Copy code
/app
   ...contents remains untouched...
/src
  /app #copied from ./app
    cli.py 
    /modes
      /mode1 
      /mode2
    /shared1
    /shared2
/test
  /app
    /modes
       /mode1
    ...contents remains untouched, apart from addition of BUILD files...
And the old
/app
is being ignored, so the relevant parts of my
pants.toml
...
Copy code
[GLOBAL]
pants_ignore.add = ["app/**"]
[source]
root_patterns = ["src", "test"]
If I wanted to 'move source out of the root' then probably I should have done this...
Copy code
/src
  /app
    /modes
      /mode1
      ...
  /test
    /app
      /modes
        /mode1
        ...
h

hundreds-father-404

03/29/2022, 11:11 PM
Instead of doing
src/app
and copying the files—which risks things falling out of sync—could you set source roots to
/
and
/test
? That will cause your production imports to be
app.modes.mode1.foo
looks like you'll still have
test
imports as
app.modes.mode1
too though because the
test/
will get stripped out, so same ambiguity you asked about in the beginning
r

rapid-exabyte-76685

03/29/2022, 11:15 PM
would it be simplest then to just leave a single source root of
/
? Not sure I am getting any benefit from multiple source roots?
Thanks @hundreds-father-404 and @happy-kitchen-89482 for your assistance BTW 🌻 🍻 🙇 🙏
❤️ 1
h

hundreds-father-404

03/29/2022, 11:19 PM
The benefit of
/
and
/test
is that it reflects how your repo actually is, if I'm understanding you correctly. It sounds lik before Pants, you import the file
app/modes/mode1/foo.py
as
app.modes.mode1.foo
. You also import
test/app/modes/mode1/test_helpers.py
as
app.modes.mode1.test_helpers
If you get rid of the source root
/test
and always use
/
, then your test code will need to be updated to import
test/app/modes/mode1/test_helpers.py
as
test.app.modes.mode1.teset_helpers
. That's probably a good change for your repo to make - it removes Pants's warning about ambiguous imports, and it also makes things simpler. But you will need to update imports, which you might not want to do yet
you're welcome!! This is definitely a tricky topic. I'm glad you reached out for help 🙂
r

rapid-exabyte-76685

03/29/2022, 11:30 PM
For the
main
branch state.... Imports within the production code under
/app
are all
app.modes.mode1.foo
to import
/app/modes/mode1/foo.py
But imports within the test code under
/test
are ... to import
/app/modes/mode1/foo.py
we import
app.modes.mode1.foo
(so same as prod code) to import
/test/app/modes/mode1/test_helpers.py
we import
test.app.modes.mode1.test_helpers
there is also a util
/test/util.py
so I think we are already in the 'good change' state in a pre-Pants world, I've just confused things by introducing separate source roots when trying to implement Pants?
h

hundreds-father-404

03/29/2022, 11:32 PM
Oh! That's great! So then you are already using
/
as your one and only source root 🙂
h

happy-kitchen-89482

03/29/2022, 11:47 PM
Aha, so that explains why things work. you already import relative to
/
so I think we are already in the 'good change' state in a pre-Pants world, I've just confused things by introducing separate source roots when trying to implement Pants?
I think this may be right. Just using
/
and keeping your sources the way they were should work fine
Pants is generally agnostic about repo layout
1