#69 Add '--exclude' arg to exclude CSV list of packages and children from output #94
Conversation
|
👍 Will check in a couple of days. |
pipdeptree.py
Outdated
| @@ -595,10 +602,15 @@ def main(): | |||
| return_code = 1 | |||
|
|
|||
| show_only = set(args.packages.split(',')) if args.packages else None | |||
| exclude = set(args.exclude.split(',')) if args.exclude else [] | |||
|
|
|||
| if show_only and exclude: | |||
There was a problem hiding this comment.
show_only and exclude can be used together as long as there are no packages in common between them. One use case of this is for eg. to show all dependencies of only "Flask" but except "Jinja2". I think it should be sufficient to validate that show_only and exclude are disjoint sets. It would be good to have a test case for this too.
tests/test_pipdeptree.py
Outdated
| @@ -122,6 +122,24 @@ def test_render_tree_list_all(): | |||
| assert 'itsdangerous==0.23' in lines | |||
|
|
|||
|
|
|||
| def test_render_tree_exclude(): | |||
| tree_str = render_tree(tree, list_all=True, exclude=['itsdangerous', 'SQLAlchemy', 'Flask', 'markupsafe']) | |||
There was a problem hiding this comment.
Can you please add a test case for exclude along with reversed tree as well?
|
Please also rebase to get latest changes on master. |
|
@naiquevin changes pushed, thanks for fb |
|
|
…es and children from output
… for py27 > py36)
|
@naiquevin thanks, last commit rebased; made both vars empty sets and added testcase for same |
pipdeptree.py
Outdated
| @@ -599,14 +605,21 @@ def main(): | |||
| if args.warn == 'fail' and (conflicting or cyclic): | |||
| return_code = 1 | |||
|
|
|||
| show_only = set(args.packages.split(',')) if args.packages else None | |||
| show_only = set(args.packages.split(',')) if args.packages else set() | |||
There was a problem hiding this comment.
Since the variable name is show_only, I think it would be better to have its default value to be None and not empty set. On the other hand, it's ok for exclude to have set() as the default value because absence of --exclude, translates to "no packages should be excluded"
The condition to show error and exit can be modified as follows,
show_only and (show_only & exclude)
There was a problem hiding this comment.
Thanks, didn't realize python stops evaluating once a condition is not met 👍
…istinct * pass args into main() to make testing easier
pipdeptree.py
Outdated
| @@ -280,8 +280,8 @@ def as_dict(self): | |||
| 'required_version': self.version_spec} | |||
|
|
|||
|
|
|||
| def render_tree(tree, list_all=True, show_only=None, frozen=False): | |||
| """Convert to tree to string representation | |||
| def render_tree(tree, list_all=True, show_only=None, frozen=False, exclude=set()): | |||
There was a problem hiding this comment.
Please use exclude=None here and handle the case inside the function. Mutable default arguments don't behave as one would expect. Check this for more info - https://stackoverflow.com/questions/1132941/least-astonishment-and-the-mutable-default-argument
There was a problem hiding this comment.
From the python docs - https://docs.python.org/3/reference/compound_stmts.html#function-definitions
There was a problem hiding this comment.
Hi @naiquevin I changed this is following commit, I'm aware of the minefield when using mutable default args, just didn't think it would matter here as render_tree is only called once, thanks 👍
pipdeptree.py
Outdated
| parser = get_parser() | ||
| args = parser.parse_args() | ||
|
|
||
| def main(args): |
There was a problem hiding this comment.
This change will break installation of the pkg because pipdeptree:main is specified as the console script entry point in setup.py. For ease of testing you can either define another intermediate function (say run) or use mocks.
|
LGTM. However, tests are failing for me locally which is probably due to some problem with my setup and not the PR. Will investigate a bit more over the weekend and then merge it. Thanks. |
|
Thank you! 👍 |
Any issues let me know, thanks