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

Add AbstractTrees Interface #72

Merged
merged 41 commits into from
Jan 11, 2024
Merged

Add AbstractTrees Interface #72

merged 41 commits into from
Jan 11, 2024

Conversation

JBlaschke
Copy link
Contributor

It exposes an AbstractTrees.jl interface via the HwlocTreeNode. Hwloc is already a tree, but it's not easy to use from Julia, so this makes things like tree traversal, and iteration, from Julia much easier.

This a follow-up for: #71

@JBlaschke JBlaschke requested a review from carstenbauer July 27, 2023 15:58
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

Attention: 55 lines in your changes are missing coverage. Please review.

Comparison is base (772b6d3) 28.36% compared to head (6d4c0e2) 33.67%.
Report is 2 commits behind head on master.

Files Patch % Lines
ext/HwlocTrees.jl 16.66% 40 Missing ⚠️
src/highlevel_api.jl 85.07% 10 Missing ⚠️
src/lowlevel_api.jl 90.19% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   28.36%   33.67%   +5.31%     
==========================================
  Files           4        5       +1     
  Lines         846      962     +116     
==========================================
+ Hits          240      324      +84     
- Misses        606      638      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@carstenbauer carstenbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments from my side:

  1. Sounds like a pretty good use case for a Pkg Extension: the tree functionality would only become automatically available when one loads both Hwloc and AbstractTrees in the same session. OTOH, AbstractTrees is very lightweight, e.g. doesn't have any dependencies, so it might be fine as is.

  2. Needs documentation, at least an example of how to use it for the README.

@JBlaschke
Copy link
Contributor Author

All done @carstenbauer -- now using Pkg Extensions. And updated README. Should I merge?

@carstenbauer
Copy link
Member

LGTM but since the README here contains the changes from PR #71 already I guess you should wait with merging until that one has landed.

BTW, I found a typo in the README: "eccident" should be "accident".

@JBlaschke
Copy link
Contributor Author

JBlaschke commented Jan 7, 2024

LGTM but since the README here contains the changes from PR #71 already I guess you should wait with merging until that one has landed.

Yep. I'll wait for your approval on #71 and #72 and then merge both in that order

BTW, I found a typo in the README: "eccident" should be "accident".

Fixed it! Thanks.

@carstenbauer
Copy link
Member

I will merge this together with #71 in about 6 hours if there are no objections.

@carstenbauer carstenbauer merged commit 6ce087c into master Jan 11, 2024
40 checks passed
@carstenbauer carstenbauer deleted the jpb/iteration branch January 11, 2024 15:48
@JBlaschke JBlaschke restored the jpb/iteration branch January 12, 2024 16:44
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.

4 participants