https://pantsbuild.org/ logo
Join Slack
Powered by
or in other words, each imports from the other (co...
# general
b
or in other words, each imports from the other (config <=> util) ?
a
so i personally have this ridiculous theory where i think circular dependencies can actually work in some cases but 99% of people find them extremely confusing so we don't support them
that would be a cycle of length 1
is it easily possible to split out the code that e.g. 
util
depends on from 
config
? or the other way around?
b
ugh... this is what happens when you go from ONE BIG LIB to monorepo of little libs...
hmm
actually
a
the output is incredibly helpful, thank you. in this case if you can move out 
ConfigEntry
from 
gh/config
into its own target, that should work
b
config wants util, but util/TEST wants config...
💡 1
a
i think you've got it
this is part of why it's nice to duplicate dependencies sometimes in parent and child targets -- makes it easier to extend to the rest of the repo
b
ok, now 
./pants dependencies --transitive gh/config
does not blow up
a
that is great to hear
b
but in general.... the deps of a tests subdir should "inherit" from the parent package dir's BUILD:dependencies, yes?
h
Are you still using 1.30 or 2.0? If you use 2.0 and use dependency inference or “explicit file addresses”, we actually do now tolerate cycles
a
it will always work to assume that a child inherits from the parent's dependencies
omg yes
it's 1.30 @hundreds-father-404
but that's a great tidbit about 2.0
b
so... what's my answer? 🙂
and should I upgrade to 2.0 ?
a
sorry:
it will always work to assume that a child inherits from the parent's dependencies
was this your question? or were you asking for best practices?
h
it will always work to assume that a child inherits from the parent’s dependencies
I don’t think this is quite the right relationship, as “parent” to me sounds like “parent directory” vs. “subdirectory”. It’s not about the relationship in the file system. Generally, depending on a dependency means that Pants will then pull in all transitive dependencies
a
ok, but "parent" is very often used to refer to trees in general, which is why filesystems use it
h
so... what’s my answer? 🙂
I’m still trying to catch up. Generally, I would recommend upgrading to 2.0. Dependency inference will do the right thing for you in most cases, so that you don’t have to manually struggle with getting dependencies right. This is a huge reason we added the feature It will also handle this cycle fine for you. If Python is fine with the cycle, Pants will be fine with it
a
and should I upgrade to 2.0 ?
yes, absolutely imho
b
ok, will swap it around to 2.0
back in a bit
👋 1
h
Because you’re still setting up Pants, I doubt there will be any big changes you need to make, beyond: https://www.pantsbuild.org/docs/how-to-upgrade-pants-2-0#step-4-upgrade-to-200 The “Key Differences To Be Aware Of” is probably irrelevant
☝️ 1
a
let us know if you have difficulties with that!
b
ERROR: Could not find a version that satisfies the requirement pantsbuild.pants==2.0
h
2.0.0b0
a
we just released a beta^
b
heh
a
sorry about that
b
"We highly recommend our beta..."
a
we do!
h
I think that’s more a “huh, that seems strange to recommend a beta rather than stable reason”. Ack. We’re close to graduating it to a release candidate, then stable. There’s some Pants history with why this is 2.0 vs. 1.30, and why we recommend 2.0 over 1.30 even though it’s a beta. For the past ~2 years, we’ve been rewriting Pants to use the the new v2 engine, which is a ground up rewrite based on the past 10 years of lessons. You were already using this v2 engine in 1.30, but Pants 1.30 also supported the v1 engine for JVM/JS/Go support, which was majorly slowing down development. We made a hard break with 2.0 to remove the v1 engine Normally, by now, we would have released 1.34.0 and you would have had these improvements like dependency inference. But 2.0 has been a drawn out release because we’re trying to get as much polish as we can before the stable release.
a
it's really sick
b
pile of Deprecation warnings
a
oh, sorry about that. hopefully they should be direct enough to fix
b
I did 
rm -rf
on the ~/.cache/pants dir and just did 
pants --version
to reinstall
h
Did you see that one section of the upgrade guide? It should knock out most of them
b
missed a few lines there
yes, now better
👍 1
Copy code
$ ./pants --version
Scrubbed PYTHONPATH=/home/mfischer/src/ghpylibs/python from the environment.
16:28:18.65 [INFO] initializing pantsd...
16:28:19.43 [INFO] pantsd initialized.
2.0.0b0
🔥 1
👖 1
h
Excellent
a
awesome
b
oh fun... tons of pytest 
[WARN]
now
and bitching about what is not mentioned in constraints.txt
a
hmmm. it probably bumps the pytest version. you should be able to set that back to the version you were using
h
So, generally, you can leave off the 
dependencies
field. Pants will infer based on your Python imports. You can run 
./pants dependencies
to see what it’s inferring You only need to fill it in when Pants cannot figure out the inference, such as when you depend on a 
files
or 
resources
target (e.g. JSON files)
b
e.g.
Copy code
16:29:20.05 [WARN] The constraints file constraints.txt does not contain entries for the following requirements: colorama, python-dateutil, mock, pytest, pytz, marshmallow
16:29:20.05 [WARN] Ignoring resolve_all_constraints setting in [python_setup] scope because constraints file does not cover all requirements.
a
ah ok
h
hmmm. it probably bumps the pytest version. you should be able to set that back to the version you were using
Yes, it does do this. See https://www.pantsbuild.org/docs/python-test-goal#pytest-version-and-plugins for how to change the version to be Pytest 5
b
Copy code
[WARN] <unknown>:587: DeprecationWarning: invalid escape sequence \{
I'm guessing that it is failing to tell me which file: <unknown> ?
(FWIW the tests did pass...)
💯 1
h
[WARN] <unknown>587 DeprecationWarning: invalid escape sequence \{
Sorry, I think that’s from dependency inference. We’ll fix this in the next beta release. Pardon the temporary noise
b
that's actually from python / pytest
I see these already under plain pytest (but it tells me the filename)
funny you should mention resource files
h
Ignoring resolve_all_constraints setting in [python_setup] scope because constraints file does not cover all requirements.
So this warning is saying that we can’t use a performance optimization that avoids having to re-resolve requirements multiple times. We use a single global resolve (like a single venv). It makes things much faster. https://www.pantsbuild.org/docs/python-third-party-dependencies#tip-set-up-a-virtual-environment-optional has a script you may find helpful to use Pants to query for your dependencies and then generate a constraints.txt You can alternatively turn off this optimization and silence the message by setting 
resolve_all_constraints = never
in 
[python-setup]
in 
pants.toml
b
yeah, I haven't gotten to getting nit-picky with that file yet
(esp. because it contains both what my repo needs and seemingly what pants itself needs)
h
I see these already under plain pytest (but it tells me the filename)
It sounds like Pytest is duplicating the warning, but yes, with the filename? Imo, Pants shouldn’t be warning in the first place. It’s warning because Pants parses the AST to be able to infer dependencies. We need to catch the DeprecationWarning
a
@hundreds-father-404 it sounds like we could maybe include that link in the error message?
h
seemingly what pants itself needs)
Are you seeing things that you don’t expect? It should only be what your own requirements are. Note that Pytest is showing up if you have 
import pytest
anywhere
b
if I run 
py.test -s gh/config/tests
it will tell me
Copy code
gh/config/tests/fig_client/test_adapter.py:56
  /src/gh-pants-test/gh/config/tests/fig_client/test_adapter.py:56: DeprecationWarning: invalid escape sequence \.
    "nested.escaped\.<http://thing.int|thing.int>": "1",

gh/config/tests/fig_client/test_adapter.py:57
  /src/gh-pants-test/gh/config/tests/fig_client/test_adapter.py:57: DeprecationWarning: invalid escape sequence \.
    "nested.escaped\.thing.str": "horse",

gh/config/fig_client/adapter.py:587
  /src/gh-pants-test/gh/config/fig_client/adapter.py:587: DeprecationWarning: invalid escape sequence \{
    delim = "\{0}".format(delim)
so I think something you're doing there is turning the filenames into 
<unknown>
👍 1
🤷
a
that is very helpful feedback
we also saw the 
/tmp/process-execution232141
prefix in the import error message from before, we should be able to scrub those error messages
b
ok, so here's something you might enjoy
I removed ALL deps from gh/config/BUILD
gh/config/tests/BUILD has only the resource files...
but it now complains that it can't find 
.exceptions
which turns out to be coming from gh/config/__init__.py doing 
from .exceptions import....
👍 1
so the tests/BUILD needs dep on "gh/config"
✍️ 1
a
that's quite interesting
b
things pointing at e.g. "gh/util" resolve without being mentioned, but something in an init parent did not
h
I got a bit lost in the threads from this morning. What did you end up having your BUILD files look like? Did you use one BUILD file per directory, or use one BUILD file for subdirectories too by using 
**/*.py
?
but something in an init parent did not
What do you mean? Is the content in the 
__init__.py
file?
b
yes
from .exceptions import ConfigException     # noqa (backwards compatibility, as ConfigException used to live here)
I think the ConfigException class used to be defined in config/__init__.py and it was moved to config/exceptions.py - so that import is for back compat callers
hmm
should the test target recursively dig down ?
h
so the tests/BUILD needs dep on “gh/config”
Taking a step back, dependencies in 2.0 are more fine-grained. Generally, dependencies are between files, like file A depends on file B. Here, we want to make sure that the correct files are depending on the correct files, rather than the “entire target”, which ends up depending on every single file, even if you only need a few of them Sorry for the confusion - this changed a bit between 1.30 and 2.0
b
e.g. I have gh/core/tests and gh/core/tests/concurrency/
(np on whatever between 1.3 and 2.0... just exploring them this past week, not deeply invested yet)
seems like the test goal did not go down from tests/ to tests/concurrency/
do I need BUILD files all the way down or is there an incantiation for the top of the tests/BUILD ?
h
I’m not following along super well. Generally, we recommend one BUILD file per directory. If you’re using dependency inference, ti would look like this for most BUILD files (if tests are colocated with source files):
Copy code
python_library()

python_tests(name="tests")
b
ok, so
I have
ns/pkg, ns/other-pkg, etc.
and each pkg has a tests subdir
there may be nestings of dirs under tests
right now I have BUILD at top of repo, under each pkg, and under each pkg/tests/
👍 1
h
You can have fewer BUILD files, but we find that it becomes hard to reason about. It’s often simplest to have one BUILD file per directory, and 1 
python_library
per directory, 1 
python_tests
per directory, etc. (Unless you have a reason to define things differently, like bump a timeout for just one test)
b
I'd have thought that 
python_library
would only be declared at the top dir OF THAT LIBRARY?
or in my example, at ns/pkg and ns/other-pkg
not in each pkg's subdirectories
h
That’s good to know that’s your expectation, I see why you’re thinking that way. And you can make it work that way, indeed. Generally, the mental model for a target is “metadata attached to some code”. When you have only one target for the entire package and subdirectories, you’re saying that every file in it has the same metadata. Often, that’s true. But, why we recommend more granularity is that it will scale better. For example, say that 
ns/pkg/subdir/f.py
depends on a JSON file. You must explicitly declare that in the 
dependencies
field for the target that refers to 
f.py
. If you have only one 
python_library
for 
ns/pkg
, then you’d end up saying that every single file in 
ns/pkg
depends on that JSON file, even though that’s likely not true. This results in worse invalidation So, having one BUILD file per directory usually a) is easier to reason about and b) scales better
b
I think we've got crossed here
I've got e.g. gh/config/tests/BUILD mentioning some json data files (because the tests use them) just fine
but the library gh.config doesn't want those
currrent Q is "why doesn't the test goal recursively go down through the 
tests/
dir ?"
i.e. it doesn't run gh/config/tests/foo/test_foo.py
only the test.py or test.py files in gh/config/tests/ are getting run
h
Right. And JSON was probably a bad example for me to use in my explanation because you are actually indeed using JSON. What I meant generally is that you will sometimes need to tweak your metadata from the default, even if the default was fine at first. If you have one BUILD file per directory, that scales much better
why doesn’t the test goal recursively go down through the tests/ dir ?”
Ah, run 
./pants test tests::
. This is a glob to say “every relevant target in tests and subdirectories”
b
yep, that seems to help
👍 1
h
(https://www.pantsbuild.org/docs/goals#file-arguments-vs-target-arguments has more about globs, including the 
:
glob for all relevant targets in the directory, but not subdirectories. And file globs with 
*
and 
**
)
So to confirm, are things working properly now? 
./pants dependencies path/to/file.py
will show you which dependencies that specific file has. It should map to what your actual imports are, and is useful when debugging things like “why is this import failing”
b
kk
well, just ran into
ns/foo/tests/ <-- has only subdirs with test files
so 
pants test ns/foo/tests::
is saying
Copy code
Unmatched globs from gh/garcon/tests's `sources` field: ["gh/garcon/tests/*_test.py", "gh/garcon/tests/conftest.py", "gh/garcon/tests/test_*.py", "gh/garcon/tests/tests.py"]
h
Okay, so if you want one single target located at 
ns/foo/tests
and for it to cover all subdirectories, you need to change the 
sources
field:
Copy code
python_tests(sources=["**/*_test.py", "**/conftest.py"])
b
hmm, removed stale pycache and pyc files, now it just returns without output
h
Or, have one BUILD file per subdirectory. Either approach works, depends on the granularity you want The default sources field will only look for the current directory. If you use something else, it needs to be changed.
b
kk, I'll tr y that
seems to be working
💯 1
(the explicit sources thing)
h
Cool, glad to hear
b
FWIW, this excercise is on a rather large repo which has lived as long as the company ....
it is opinionated on its layout, as are its maintainers so... 🙂
h
Were y’all using a build tool before?
b
fewer BUILD files is an aim of this experiment
for this lib? Not really
CI => 
tox --
and "Build" 
python setup.py.....
but the desire has been expressed for it to be broken up into a monorepo of ns.core, ns.util, ns.foo, etc..
so that folks can have more granular installs of the deps of apps which use this lib
h
It is opinionated on its layout, as are its maintainers
We generally make it a goal for Pants to work with multiple different workflows / layouts. For example, allowing you to set source roots If there are particular features that would help you all out, please let us know. A lot of functionality is driven specifically by user interest. For example, a user wanted to be able to indicate source roots based on the presence of a marker file (
setup.py
). We had never used that workflow, but added it to support their layout. In other words, we don’t want you to have to rewrite your whole project to use Pants 😄 (Although, we hope that it will help encourage good practices at the same time, and bring you benefits like hermiticity, caching, and concurrency)
fewer BUILD files is an aim of this experiment
Cool. This was a major reason for two changes we made in 2.0: a) dependency inference, and b) being able to depend explicitly on files, rather than having to depend on the entire target and pulling in every single file. While I was advocating “one BUILD file per directory”, it is totally possible and valid to have fewer BUILD files like you’re doing now in 2.0. That was not really feasible in 1.x where you had to explicitly declare dependencies, but is now very valid
python setup.py.....
If you haven’t yet, check out https://www.pantsbuild.org/docs/python-setup-py-goal when you’re ready. With 
./pants setup-py
, you will only define one 
python_distribution
target per package, like you’d expect. Let us know if we can help with getting that going
b
what is the diff between 
python_distribution
and 
python_library
?
I could swear I tested 
bdist_wheel
with this and used 
python_library
and not distribution....
h
A python_distribution is metadata about how to package and distribute your code via the 
setup-py
goal. You can set things like the 
version
and 
description
Meanwhile 
python_library
is metadata about any Python source files you have.
b
ah. mmm
h
Ah yes, in 1.30, you used to define the 
provides
field on 
python_library
. 
python_distribution
is new to 2.0 to clean up the modeling Again, sorry for the confusion of switching between them. We only changed the docs to default to 2.0 on Friday because we had to rewrite some parts to talk about things like dependency inference. So if you started Pants earlier, that explains why you were using 1.30
b
indeed, got started on this about a week ago
👍 1
ok, so in say... gh/core (a lib we want to publish)
I'd have both 
python_distribution
and 
python_library
stanzas?
or just distribution?
hmm, sees I tweaked the python_library.sources earlier today to get the */.py thing to chase down nested files
so likely both?
h
I’d have both python_distribution and python_library stanzas?
Yep, you’d have something like this:
Copy code
python_library(
  name="lib", 
  sources=["**/*.py"]
)

python_distribution(
  name="dist"
  dependencies=[":lib"],
  provides=setup_py(
     name="gh-config-dist",
     version="1.0.1",
     ...
   )
)
The 
python_library
is attaching metadata to your actual 
.py
files. For example, what dependencies each 
.py
file has. Or, what 
compatibility
it has for Python versions Meanwhile, 
python_distribution
is metadata about your distribution you want to build
b
$ ./pants setup-py gh/core
h
Note that 
python_distribution
does not work with dependency inference. Why? It doesn’t have any source files, so Pants can’t analyze imports. Instead, you use an explicit dependency Here, I’m assuming that you want to depend on “everything in the 
lib
target, and everything that those files in the target depends on too”
b
Copy code
TargetNotExported: Cannot run setup.py on these targets, because they have no `provides=` clause: gh/core/collections.py, gh/core/concurrency/__init__.py:../core, gh/core/log.py, gh/core/logging/formatter.py:../core, gh/core/property.py, gh/core/tests/concurrency/executor_test.py:../../core, gh/core/util.py, gh/core/ioc/service_locator.py:../core, gh/core/retry.py, gh/core/__init__.py, gh/core/ioc/__init__.py:../core, gh/core/ioc/exceptions.py:../core, gh/core/tests/retry_test.py:../core, gh/core/logging/__init__.py:../core, gh/core/tests/collections_test.py:../core, gh/core/tests/ioc/service_locator_test.py:../../core, gh/core/patterns.py, gh/core/concurrency/executor.py:..
h
Run 
./pants setup-py gh/core:dist
Or, if you would rather be able to do 
./pants setup-py gh/core
, you can leave off the 
name
field for the 
python_distribution
target. If you leave off 
name
, it will default to the name of the directory
(Running 
./pants setup-py gh/core
is shorthand for 
./pants setup-py gh/core:core
)
b
hmm
Copy code
./pants setup-py gh/core:dist
17:39:37.01 [ERROR] 1 Exception encountered:

  ResolveError: 'dist' was not found in namespace 'gh/core'. Did you mean one of:
  :core
  :gh.core
h
what does your BUILD file look like?
b
Copy code
python_library(
    sources=['**/*.py'],
    dependencies=[],
)

python_distribution(
    name="gh.core",
    dependencies=[ ],
    provides=setup_py(
        name="gh.core",
        version="0.1",
        description="this is gh.core",
        author="mycorp",
        classifiers=[
            "Programming Language :: Python :: 3.6",
        ],
        namespace_packages=['gh.core'],
    )
)
h
Ah, sorry, miscommunication I think. What you have here is that 
python_library
has the address of 
gh/core:core
, which you can also use 
gh/core
as shorthand. This is because you left off the 
name
field. You explicitly set 
name
to 
gh.core
for the 
python_distribution
, so its address is 
gh/core:gh.core
-- I suspect that instead, you want to leave off 
name
for 
python_distribution
. This will result in the address 
gh/core:core
, which you can use 
gh/core
as shorthand. But, if you do this, you need to add something like 
name="lib"
to the 
python_library
. Each address must be unique, so you can only have one target in each directory that leaves off the 
name
field
b
but the setup_py(name="gh.core") should remain, yes?
(FWIW
Copy code
ValueError: namespace_packages cannot be set in the `provides` field for gh/core, but it was set to ['gh.core']. Pants will dynamically set the value for you.
)
h
Yes, that 
setup_py(name="gh.core")
is the name of the actual distribution, like what the wheel will be called. It’s different than Pants’s address to the 
python_distribution
target. That’s pretty confusing. I think it might make more sense if we called it 
dist_name
? Generally, that 
setup_py
thing uses the same keyword args you would use in a 
setup.py
, which is why we stuck with 
name
. But it could be worth diverging here.
b
so,
./pants setup-py gh/core
worked, but it created an empty dist/gh.core-0.1/src/ dir....
h
Because 
dependencies
is empty. Add 
dependencies=[":lib"]
b
and lib.sources is where I would filter out the tests dirs from the pkg, yes?
h
Yes, exactly. You’ll want 
gh/config:lib
to have a sources like 
sources=["**/*.py", "!**/*_test.py"]
You can run 
./pants dependencies --transitive gh/config
to spot check what is going to end up in the 
python_distribution
b
to leave out the whole tests dir, ["**/*.py", "!tests"] ?
seems to have worked
now the killer test, bdist_wheel, install and import it in ipython
ouch
ERROR: gh.core-0.1-py2-none-any.whl is not a supported wheel on this platform.
👀 1
h
What command did you run? You may want to do 
./pants setup-py gh/confg -- bdist_wheel --python-tag=py36.py37
for example
b
./pants setup-py gh/core -- bdist_wheel
what does 
--python-tag
do here?
(it "worked" at any rate)
wooohoo
🔥 1
Copy code
In [1]: import gh.core                                                                                               

In [2]: from gh.core.log import class_logger
h
That’s a flag to 
bdist_wheel
. Anything after the 
--
gets passed through It tells the 
bdist_wheel
program that it’s compatible with Py36 and Py37, for example I’m starting to get out of my expertise, though. @happy-kitchen-89482 do you know why Michael is having to set 
--python-tag
to get the wheel built properly? (Only the last few messages are relevant, you can ignore most this thread)
b
I was thinking it was missing what usually goes in setup.cfg, eg.
Copy code
[bdist_wheel]
universal=1
but I too am out of my depth there
h
Ahh, yes. That’s what’s going on. That is an alternate way to tell 
bdist_wheel
what this is compatible with
b
Much progress today - Thank you!
I need coffee, and then to test the 
test --changed-since=origin/master
trick with 2.0
(which was honestly the biggest selling point in my org - saves us having to write "which pkgs need to be retested?" code ourselves)
h
You might be able to define a 
resources
target for your 
setup.cfg
file and add it to the 
dependencies
field of your 
python_distribution
. I suspect it would fix it We are also considering redesigning/improving some of the 
setup-py
stuff so that you can associate certain passthrough arguments permanently to a 
python_distribution
target
then to test the test --changed-since=origin/master trick with 2.0
Yay! One of my favorite features. I use 
./pants --changed-since=HEAD fmt
all the time For 
test
, often you will want to use 
--changed-dependees=transitive
. This means that if you change source file a.py, and b.py depends on it, then test_b.py will rerun because it knows a transitive dep changed Also, you may find 
./pants --changed-since=origin/master list
helpful when experimenting with the behavior. That will list every file that is picked up, without the cost of actually running tests
b
yep, I found it to work on your example repo before, but need to retest now wth the current code/setup/version
💯 1
oh, nice point about the 
list
Q - is it clever enough that if I change core, (assume that every other lib in the repo depends on core) - it will say "oh, I need to test all of those as well"?)
h
Yes, iff you use the 
--changed-dependees=direct
or 
--changed-dependees=transitive
option though The default is 
--changed-dependees=none
, which is only the changed files themselves
b
hmm
./pants  --changed-since=master --changed-dependees=transitive list
and it listed I think, the whole repo tree
(after I made one trivial change to a single file in gh/util/date.py)
yeah, something wrong there, if I do 
test
it ist testing things which are not on the graph with that file
h
Do you still have explicit dependencies anywhere other than for the JSON files and Python distribution target? Those dependencies might be much coarser than you want
You may find the dependees goal useful to see what is depending on that file
Use 
./pants dependees path/to/f.py
to see everything that is directly depending on it. Does that look right? Then, you can use 
dependees --transitive
as the next step
b
ugh, yes,
possibly a bad choice of mine to play with such a used file
🙃 1
why would 
dependees
be showing me
Copy code
18:26:11.56 [WARN] <unknown>:587: DeprecationWarning: invalid escape sequence \{
w
i don’t think that that is related to 
dependees
, per-se… but i don’t think we’ve triaged it yet.
in particular, whether that escape sequence is in pants’ code, or end user code. i’ll take a quick look.
b
if I run 
pytest
over the same code, those warnings are eemitted, but with the filename rather than 
<unknown>
w
ah, good to know… is the reported filename in your code, or in pants?
h
That’s the dependency inference AST parsing that doesn’t capture depreciation warnings. What I was mentioning much higher up in the thread. We need to fix that @witty-crayon-22786 originally we did catch it, but iirc you were concerned it wasn’t thread safe Alternatively, we could fix Pants to say what the file name is
w
using a contextmanager to catch it is not threadsafe, correct… there might be other methods.
but yea, i remember now: thank you.
👍 1
b
the files whose code produces the warnings is my code, not yours 🙂
👍 1
h
We definitely need to fix. It’s annoying because there’s no remedy atm; you can’t see the issue file My vote personally would be to catch the deprecation. Pants’s dependency inference imo shouldn’t have the side effect of logging warnings. But if we can’t do that, fixing to say what the file name is would be good
w
so then in this case, perhaps we can tell the AST module which file we’re evaluating so that the warning is raised “from somewhere”
yea. the thread safety issue just gets a big shrug apparently: https://docs.python.org/3/library/warnings.html#warnings.catch_warnings
😕 1
h
Exactly. That’s what im thinking. We’ll still pass it a string rather than file, but give a hint for what the file name is
w
i’ll file.
h
Oh, should we just globally ignore this warning no matter what? Set it up in our initial pants runner file. I don’t think Pants should ever warn about this in any context. For our own code, Pytest will use its own options filtering and will still print it. Ditto when users run tests on their own code. Sub processes can still warn; this is only about the main Pants process
w
presumably it’s not the only warning that is possible in the context of AST parsing
h
True. Like deprecation warnings when you are using collections instead of collections.abc But still, I don’t think we want to warn there either. The subprocess should, but not Pants itself
b
easy way to repro : put 
varname = "foo\bar"
in your code somewhere
💯 1
like a regex pattern NOT preceded by 
r
w
h
This was pretty trivial to fix: https://github.com/pantsbuild/pants/issues/10826 Thanks Michael for reporting this
b
👍
4 Views
Open in Slack
PreviousNext
Powered by