https://pantsbuild.org/ logo
Powered by
l

lemon-oxygen-72498

11/23/2022, 5:03 PM
I have a python monorepo with a layout like this:
Copy code
.
└── libs
    ├── a
    │   ├── BUILD
    │   ├── kaiko
    │   │   └── a <- *.py files in there
    │   └── requirements.txt
    ├── b
    │   ├── BUILD
    │   └── kaiko
    │       └── b <- *.py files in there
    └── c
        ├── BUILD
        └── kaiko
            └── c <- *.py files in there
So in the library 
a
- that depends on both 
b
and 
c
- we have imports of the from 
from kaiko import b
and 
from kaiko import c
, because 
kaiko
is a shared namespace. Both 
libs/a
, 
libs/b
, and 
libs/c
are roots. `a`'s 
requirements.txt
contains editable installs stanzas (
-e ../b
and 
-e ../c
). That doesn't work when typechecking with 
mypy
(and 
pyright
), I get 
Cannot find implementation or library stub for module named "kaiko"
and 
Cannot find implementation or library stub for module named "kaiko.b"
errors 😞💥 🏴‍☠️ (depending on the import's shape). Should I use modules and module mapping statements in my 
BUILD
file to circumvent this? I clearly see that my setup seems not to fit w.r.t. how pants treats imports, but I don't understand how I would write the mappings.
b

bitter-ability-32190

11/23/2022, 5:04 PM
Looks like 
kaiko
is an implicit namepsace package?
You can try making it an explicit namespace package
l

lemon-oxygen-72498

11/23/2022, 5:05 PM
I'm not fluent with python jargon, but I believe yes. I have this in my `pyproject.toml`:
Copy code
packages = [
  { include = "kaiko" }
]
b

bitter-ability-32190

11/23/2022, 5:05 PM
l

lemon-oxygen-72498

11/23/2022, 5:06 PM
You can try making it an explicit namespace package
What does this entail exactly?
b

bitter-ability-32190

11/23/2022, 5:06 PM
The 
__init__.py
file for the namespace package needs to contain only the following:
Copy code
__path__ = __import__('pkgutil').extend_path(__path__, __name__)
l

lemon-oxygen-72498

11/23/2022, 5:09 PM
Humm but some 
__init__.py
files already contain things 🤔
I need to log off now, but thanks for the suggestion 👍 Will continue tomorrow.
b

bitter-ability-32190

11/23/2022, 5:10 PM
This would be in just the 
__init__.py
file immediately underneath any 
kaiko
directory
h

happy-kitchen-89482

11/23/2022, 5:11 PM
Pants will ignore editable installs in requirements.txt, as it already knows how to knit together in-repo deps
But I hope that is not the problem
and yes, 
kaiko
is a namespace package, if any of those 
kaiko
dirs has an 
__init__.py
then they all must, and it must contain that line Josh posted
if none of them do, then it should work as an implicit namespace
b

bitter-ability-32190

11/23/2022, 5:14 PM
if none of them do, then it should work as an implicit namespace
This trips up 
mypy
though 😕
l

lemon-oxygen-72498

11/23/2022, 7:24 PM
Thanks for the explanations filling the gaps of my python-knowledge ❤️ So I confirm we are using implicit namespace packages: 
libs/a/kaiko
directories don't have a 
___init___.py
file (
libs/a/kaiko/a
does).
I must mention that the sandbox creation is correct: it reproduces the 
libs
structure I have outlined in my message on #general, bringing in 
libs/{b,c}/kaiko/{b,c}/**.py
when typechecking 
a
.
h

happy-kitchen-89482

11/23/2022, 8:52 PM
I guess it’s just that mypy is difficult then…
b

bitter-ability-32190

11/23/2022, 10:27 PM
And pyright, I believe
l

lemon-oxygen-72498

11/24/2022, 3:01 PM
I have made a minimal example:
Copy code
> git clone <https://github.com/smelc/pants-implicit-namespaces-typecheck>
> cd pants-implicit-namespaces-typecheck
> PANTS_SHA=a4054868c11690874005999cb80100e4da107538 ./pants check ::
Could someone have a look? I also posted the output in the repo's README: https://github.com/smelc/pants-implicit-namespaces-typecheck
b

bitter-ability-32190

11/24/2022, 3:20 PM
Did you try making them explicit namespace packages. I suspect that fixes the issue
l

lemon-oxygen-72498

11/29/2022, 10:10 AM
That didn't help 😞 I'm running out of ideas, if anyone could make 
pyright
work in the repo https://github.com/smelc/pants-implicit-namespaces-typecheck (see the detailed description in the repo's README.md), that would be tremendously helpful to me. cc @wide-midnight-78598
w

wide-midnight-78598

11/29/2022, 11:28 AM
Hmm, this is bizarre - I'll take a look at this in a few hours
❤️ 1
🙏 1
I have made a minimal example:
Oh 5 days ago? Sorry - I missed this message entirely
l

lemon-oxygen-72498

11/29/2022, 11:36 AM
No worries, I have been busy too on other fronts!
w

wide-midnight-78598

11/29/2022, 11:41 AM
Well, both good and bad:
Copy code
npx pyright@1.1.258 .

No configuration file found.
No pyproject.toml file found.
stubPath /Users/sj/Developer/scratch/pants-implicit-namespaces-typecheck/typings is not a valid directory.
Assuming Python platform Darwin
Searching for source files
Found 3 source files
pyright 1.1.258
/Users/sj/Developer/scratch/pants-implicit-namespaces-typecheck/libs/a/mycorp/a/a.py
  /Users/sj/Developer/scratch/pants-implicit-namespaces-typecheck/libs/a/mycorp/a/a.py:1:20 - error: "b" is unknown import symbol (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations 
Completed in 0.526sec
So, while the error isn't great, at least 
pyright
in and out of pants react the same
I have an idea - will report back shortly, just out of the house for a few hours
👍 1
l

lemon-oxygen-72498

11/29/2022, 11:44 AM
It's weird you have the import error out of 
pants
😐 I don't have have it. Did you run 
pip install -r requirements.txt
in 
libs/a
before running 
pyright
? (doing so will install 
-e ../b
)
w

wide-midnight-78598

11/29/2022, 12:30 PM
Intentionally didn't do it - wanted to see if it was a venv issue
👍 1
There are a few things going on here, the requirements is an editable install from 
libs/a/requirements.txt
? While the 
pants.toml
is at the root. As Benjy mentioned above, "Pants will ignore editable installs in requirements.txt", so that should be ignored. Now, the question is why Pants isn't picking it up itself via dep inference Source roots are:
Copy code
PANTS_SHA=a4054868c11690874005999cb80100e4da107538 ./pants roots
.
libs/a
libs/b
Which I believe should be correct. Checking out all our targets:
Copy code
PANTS_SHA=a4054868c11690874005999cb80100e4da107538 ./pants list ::                                                                                                                                      ⎇ main*
libs/a:mycorp-a
libs/a/mycorp/a/a.py:../../mycorp-a
libs/a/mycorp/a:a
libs/a/mycorp/a/a.py
libs/b:mycorp-b
libs/b/mycorp/b/__init__.py:../../mycorp-b
libs/b/mycorp/b/b.py:../../mycorp-b
libs/b/mycorp/b:b
libs/b/mycorp/b/__init__.py
libs/b/mycorp/b/b.py
mycorp/a/
doesn't have an initpy - should it? https://github.com/smelc/pants-implicit-namespaces-typecheck/tree/main/libs/a/mycorp/a The weird part for me is that there are two sets of BUILD files referring to the same files: https://github.com/smelc/pants-implicit-namespaces-typecheck/blob/main/libs/a/BUILD https://github.com/smelc/pants-implicit-namespaces-typecheck/blob/main/libs/a/mycorp/a/BUILD Ditto for B, so removing one of those should help with the warnings Transitive deps on a.py
Copy code
PANTS_SHA=a4054868c11690874005999cb80100e4da107538 ./pants dependencies --transitive libs/a/mycorp/a/a.py
libs/b/mycorp/b/__init__.py:../../mycorp-b
Transitive deps on a.py after explicitly adding 
dependencies=["libs/b:mycorp-b"]
to A's BUILD
Copy code
PANTS_SHA=a4054868c11690874005999cb80100e4da107538 ./pants dependencies --transitive libs/a/mycorp/a/a.py
libs/b/mycorp/b/__init__.py:../../mycorp-b
libs/b/mycorp/b/b.py:../../mycorp-b
l

lemon-oxygen-72498

11/29/2022, 2:45 PM
the requirements is an editable install from 
libs/a/requirements.txt
Yes correct (this is a development setup)
w

wide-midnight-78598

11/29/2022, 2:47 PM
And as a sanity, 
mypy
also fails on this - 
pyright
plugin shares a bunch of 
mypy
code, so its a good check (this might be a configuration problem though, with implicit vs explicit namespaces, I always have to read the docs for that):
Copy code
09:47:12.81 [INFO] Completed: Building mypy.pex from mypy_default.lock
09:47:14.00 [INFO] Completed: Building requirements_venv.pex
09:47:23.33 [ERROR] Completed: Typecheck using MyPy - mypy - mypy failed (exit code 1).
libs/a/mycorp/a/a.py:1: error: Cannot find implementation or library stub for module named "mycorp"
libs/a/mycorp/a/a.py:1: note: See <https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports>
Found 1 error in 1 file (checked 3 source files)
As another sanity, I changed b's 
mycorp
to 
mycorp1
to see what error is spit out: 
libs/a/mycorp/a/a.py:1:6 - error: Import "mycorp1" could not be resolved (reportMissingImports)
Okay, so a few things to get 
pyright
working here: I think the constant uses of 
a
and 
b
really messed me up - so this took a while:
Copy code
from b.mycorp.b import b

def add3(x: int, y: int, z: int) -> int:
    return b.add(x, y) + z
It looks like there is a 
b
module in the 
b
package of 
mycorp
which is in 
b
from the perspective of Pants's Pyright.
In summary, I hate implicit namespaces...
@happy-kitchen-89482 ^ What am I missing here? With a source root of 
libs/b
shouldn't that first 
b
package be dropped?
@lemon-oxygen-72498 I have to dig around in 
pyright
but my guess is that there are some more configurations that need to be passed in via the pyrightconfig.json or pyproject.toml
Specifically - maybe even just adding the source roots as pyright's "includes":
Copy code
{
  "include": [
    "libs/a",
    "libs/b",
    "."
  ]
}
Doesn't work on it's own, but hopefully in conjunction with other settings - or maybe 
extraPaths
- that appears to work. Will open a ticket for finally getting 
pyright
configurations handled in the tool. Some parts of config need to be overridden, so it's a bit messy
l

lemon-oxygen-72498

11/29/2022, 8:25 PM
Great to see you have made progress ❤️ Let me know when I can test something to help you 👍 For the record I also supposed that 
pyright
was picking something up from 
pyproject.toml
, but pyright's documentation states that it only inspects things under the 
[pyright]
entry and I have nothing namespace specific in my real life scenario, my 
[pyright]
section is as follows:
Copy code
[tool.pyright]
reportMissingParameterType = true
strictListInference = true
strictDictionaryInference = true
strictSetInference = true
I have also tried creating 
pyproject.toml
in `pants`' sandox, and running 
__run.sh
manually, thinking it could maybe pick some stuff from 
[tool.poetry]
(such as 
packages = [  { include =  mycorp" } ]
) but I couldn't trigger a change in `pyright`'s behavior when doing so 🤷
So to sum up, it seems 
pyright
indeed only reads the 
[pyright]
entry from 
pyproject.toml
🙂
w

wide-midnight-78598

11/29/2022, 8:36 PM
Yep, cool beans. There is an outstanding question about how to apply the merged Pants + Pyright configs, which I'll need to look at a bit
Hoping to take a crack at it this week
❤️ 1
@lemon-oxygen-72498 if you could take a look and see if this works? https://github.com/pantsbuild/pants/pull/17771 I added configs that get patched with the 
venv
and the 
extraPaths
(source roots). Makes your sample repo pass. I have some cleanup, docs, and re-factoring, but high-level seems to be fine
❤️ 1
l

lemon-oxygen-72498

12/11/2022, 2:26 PM
Hey @wide-midnight-78598, thanks for the work! I will try in the next days.
w

wide-midnight-78598

12/11/2022, 2:27 PM
No problem, want to make sure it works on your actual repo as well, not just the sample one. There will be a broader config PR down the road which also captures execution environments, which I think we’ll want for interpreter constraints, or monorepos
👍 1
l

lemon-oxygen-72498

12/11/2022, 2:28 PM
Sounds good, I'm visiting my company's office tomorrow and the day after tomorrow, so my schedule is unusual. But will try asap on our true trop to confirm 🙂
w

wide-midnight-78598

12/14/2022, 2:01 PM
Can you try 
./pants_from_sources roots
To make sure the source roots are correct
l

lemon-oxygen-72498

12/14/2022, 4:44 PM
@wide-midnight-78598> yes they are. I get this on my sample repo:
Copy code
→ ./pants_from_sources roots
.
libs/a
libs/b
and a correct list on my production repo. In both cases I have 
marker_filenames = ["pyproject.toml"]
.
w

wide-midnight-78598

12/14/2022, 4:48 PM
Hmm, I'll look at this tonight - it passed on your sample repo, so I'm confused
l

lemon-oxygen-72498

12/14/2022, 4:56 PM
Yeah I am too, I double checked my sample repo example, 
git diff
is clean, source 
pants
repo is at the correct SHA, etc. 🤷 Would it be useful if I zip the generated sandbox when it fails? (so that you compare with your sandbox)
Here it is (removed the 
node-v16.15.0-linux-x64
folder from it, to save space)
pyight_pants_faulty_sandbox_smelc.zip
w

wide-midnight-78598

12/15/2022, 2:09 AM
Okay, so the fact that 
extraPaths
isn't there is a surprise. I'd wager that the wrong version is used or something
Well, this is embarrassing... I forgot to push the commit that included the "no config" extraPaths usecase 🤦‍♂️
5 Views
Powered by