search

ytree-project / ytree

A yt-based merger-tree code.
Other
15 stars 10 forks source link

Add TreeFrog frontend #88

Closed brittonsmith closed 3 years ago

brittonsmith commented 3 years ago

PR Summary

This adds support for loading TreeFrog merger trees (Issue #66).

Saving an entire arbor is a little slow because reading full trees requires reading from multiple HDF5 groups. I'll think about optimizations, but I think this is ready to go in as is now.

@manodeep, I added you as a reviewer just to get your attention. Don't feel you need to review, but any comments certainly welcome.

PR Checklist

brittonsmith commented 3 years ago

@manodeep I'm going to merge this, but if you have any comments about anything, we can fix it up later.

manodeep commented 3 years ago

@brittonsmith Apologies that I haven't managed to take a look yet - I do intend to and report back

manodeep commented 3 years ago

I did a quick skim but did not poke into the details of io.py - looked too intimidating. Is it possible to add some inline code comments to io.py

brittonsmith commented 3 years ago

I did a quick skim but did not poke into the details of io.py - looked too intimidating. Is it possible to add some inline code comments to io.py

I will add some comments and issue a new PR including changes from your other comments.

brittonsmith commented 3 years ago

@manodeep I've opened PR #95 with changes as per your comments.