-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update MAG class to MetaBin #67
Conversation
* resolves issue-KwanLab#54 * 📝 Update docstrings missing for functions. * 🐎 Add cache for properties reading assembly sequences. * 🎨 Add Entrypoint functionality for incorporation to packaging/distribution. * 🎨🐛 Update main() to handle updated functions.
* 📝 Add docstrings to methods and class * 🎨🔥 Remove split_nucleotides method. * 🎨 Update get_binning function to handle coverages as filepath not pd.DataFrame * 🎨 Change mag.py to metabin.py * 🐎 fixed from PR-KwanLab#66. Add cache functionality to time-consuming property methods. * 📝 Add COMBAK comment for checkpointing. Return to this when implemented in utilities.py
autometa/common/metabin.py
Outdated
f'Num. Nucl. ORFs: {self.nnucls:,}\n' | ||
f'Num. Prot. ORFs: {self.nprots:,}\n' | ||
f'Size: {self.size:,}bp / {self.totalsize:,}bp ({self.size_pct:4.2f}% of total metagenome)\n' | ||
f'Mean GC content: {self.length_weighted_gc:4.2f}%\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The print output should probably explicitly say the GC content is length-weighted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I just had one comment. I like the docstrings for properties in metabin.py
. For consistency it would be good to have them in metagenome.py
, either as part of this PR or PR #66.
* 📝🎨 Add defaults formatter_class to template.py
* 🎨 Change os.stat(fpath).st_size to os.path.getsize(fpath)
I recommend addressing #66 prior to this. |
The comment above about the |
This was addressed in commit 3033b0d The changes should be reflected in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks fine to merge now
user.py
forget_binning
rather thanpd.DataFrame
.mag.py
tometabin.py
.utilities.py
.