Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for arbitrary kwargs in tree(...) #13

Merged
merged 2 commits into from
May 19, 2024

Conversation

ReubenJ
Copy link
Contributor

@ReubenJ ReubenJ commented May 8, 2024

This change allows for users to control features like the maxdepth of the tree. It is also a more robust solution than, for example, simply adding a maxdepth argument to tree(...) itself.

Without maxdepth:

Output

julia> PkgDependency.tree("Term")
Term 2.0.6                                       
  ├─ ProgressLogging v0.1.4                      
  │  └─ SHA v0.7.0                               
  ├─ CodeTracking v1.3.5                         
  ├─ PrecompileTools v1.2.1                      
  │  └─ Preferences v1.4.3                       
  │     └─ TOML v1.0.3                           
  ├─ OrderedCollections v1.6.3                   
  ├─ MyterialColors v0.3.0                       
  ├─ Tables v1.11.1                              
  │  ├─ DataAPI v1.16.0                          
  │  ├─ OrderedCollections v1.6.3 (*)            
  │  ├─ IteratorInterfaceExtensions v1.0.0       
  │  ├─ DataValueInterfaces v1.0.0               
  │  └─ TableTraits v1.0.1                       
  │     └─ IteratorInterfaceExtensions v1.0.0 (*)
  ├─ UnicodeFun v0.4.1                           
  ├─ AbstractTrees v0.4.5                        
  ├─ Parameters v0.12.3                          
  │  ├─ OrderedCollections v1.6.3 (*)            
  │  └─ UnPack v1.0.2                            
  └─ Highlights v0.5.2                           
     └─ DocStringExtensions v0.9.3

With maxdepth=1, for example:

Output

PkgDependency.tree("Term", maxdepth=1)
Term 2.0.6                    
  ├─ ProgressLogging v0.1.4                        
  │                           
  ├─ CodeTracking v1.3.5      
  ├─ PrecompileTools v1.2.1                        
  │                           
  ├─ OrderedCollections v1.6.3
  ├─ MyterialColors v0.3.0    
  ├─ Tables v1.11.1                        
  │                           
  ├─ UnicodeFun v0.4.1        
  ├─ AbstractTrees v0.4.5     
  ├─ Parameters v0.12.3                        
  │                           
  └─ Highlights v0.5.2        
                             

I've added tests for the maxdepth keyword and updated the documentation to highlight the added functionality. The print_tree documentation is currently not published, so I've omitted that link (waiting on JuliaCollections/AbstractTrees.jl#145).

This change allows for users to control features like the `maxdepth` of the tree.
It is also a more robust solution than, for example, simply adding a `maxdepth`
argument to `tree(...)` itself.
@peng1999
Copy link
Owner

peng1999 commented May 9, 2024

Thanks for the contribution! I see the value of the maxdepth option. However, are there any other useful options in Term.Trees.Tree or print_tree? If maxdepth is the only option users need, we could offer just that instead of forwarding all arguments.

There is another concern regarding the maxdepth option. Consider a tree like this:

Root
  ├─ PkgA
  │  └─ PkgB
  │     └─ PkgC
  └─ PkgB (*)

If you set maxdepth=1, the first instance of PkgB will be omitted. This might seem confusing because this leads to a (*)-marked package without any preceding node displayed. However this can be work around by setting dedup=false. We can move forward if you feel fine with this.

@ReubenJ
Copy link
Contributor Author

ReubenJ commented May 9, 2024

Good points! I don't have an immediate answer to the question of hidden duplicate packages.

I could see some of the other options of print_tree being useful in some niche cases. Also, if there are some new, useful options introduced in the future, we wouldn't need to change anything here. Just an idea—I'm not attached to keeping this solution.

@peng1999
Copy link
Owner

Sorry for the late response. I did some research but did not found a better solution. So I think it’s better to merge this PR for now. Thank you for the contribution!

@peng1999 peng1999 merged commit 3728d05 into peng1999:master May 19, 2024
6 checks passed
@ReubenJ ReubenJ deleted the feature/arbitrary-kwargs branch May 24, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants