pants: Support `with suppress(ImportError):` in python dependency inference

Is your feature request related to a problem? Please describe. Currently the python dependency inference of pants takes into account any imports within a try/except ImportError block and marks them as “weak” dependencies, e.g. skipping the warning if they are not provided by any requirement. Using the equivalent with contextlib.suppress(ImportError):, the dependencies are marked as “strong” and a warning is printed.

Describe the solution you’d like Detect with contextlib.suppress(ImportError): or from contextlib import suppress; ...; with suppress(ImportError): in the dependency parser and handle those dependencies as weak.

Describe alternatives you’ve considered Alternatively, to allow more freedom for special cases in user code, make the dependency parser configurable either through options of python-infer or a plugin interface.

I experimented a bit with the latter, but could not override the default dependency parser without too much boilerplate code or wrapping of pants classes. However this might be because I’m not familiar with writing plugins.

Additional context Tested with pants 2.14.0.

Please give feedback on what solution would be preferable.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 1
  • Comments: 15 (11 by maintainers)

Commits related to this issue

Most upvoted comments

We would be looking for exactly this:

with suppress(ImportError):
  ...

I think the intention there is very unambiguous. If that does not, in fact, suppress the import error, then the author is playing with fire…

This is a good suggestion! It might not be too hard to modify the dep parser to do this, if you want to take a swing at it. The crucial file is https://github.com/pantsbuild/pants/blob/68894b9918d4a1d4802888238f03cbc0c68e9bfd/src/python/pants/backend/python/dependency_inference/scripts/dependency_parser_py

The important thing to me is that we’d support both import contextlib + contextlib.suppress and from contextlib import suppress + suppress

It would be easy to check for with suppress(ImportError) or with contextlib.suppress(ImportError) locally, or even add a check for import contextlib / from contextlib import suppress in the top-level statements.

However, I think that protecting from all possible complications goes too far and would be out of scope for this issue. I mean things like making sure the correct suppress has been imported (e.g. if someone first does from contextlib import suppress and later from anotherlib import something as suppress) and that the scoping is correct (e.g. if contextlib was only locally imported in some other scope). What do you think?