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

feat: add repr to dictlist #1278

Draft
wants to merge 8 commits into
base: devel
Choose a base branch
from

Conversation

achillesrasquinha
Copy link
Contributor

It would be nice to have a neater representation of the model objects - metabolites, reactions, genes, etc. For example,
Screen Shot 2022-09-18 at 11 15 21 PM

@achillesrasquinha achillesrasquinha changed the title feat: add gene info to repr html feat: add repr to dictlist Sep 19, 2022
@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2022

Codecov Report

Attention: Patch coverage is 39.13043% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 84.13%. Comparing base (b438d2f) to head (28376e6).
Report is 70 commits behind head on devel.

❗ Current head 28376e6 differs from pull request most recent head 5b45b1f. Consider uploading reports for the commit 5b45b1f to get more accurate results

Files Patch % Lines
src/cobra/core/dictlist.py 17.64% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #1278      +/-   ##
==========================================
- Coverage   84.40%   84.13%   -0.28%     
==========================================
  Files          66       66              
  Lines        5497     5520      +23     
  Branches     1265     1269       +4     
==========================================
+ Hits         4640     4644       +4     
- Misses        551      570      +19     
  Partials      306      306              

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

@cdiener
Copy link
Member

cdiener commented Sep 19, 2022

An alternative approach would be to have DictList inspect its own element type and map the printed columns based on that. Not great for encapsulation, but that does not break the pickling of models. Not sure what would be the best strategy.

@achillesrasquinha
Copy link
Contributor Author

I thought on similar lines as well is what attributes would go in best. But I think sticking to this style made me feel that reliance on instance specific df wouldn't be so concerning.

@Midnighter
Copy link
Member

I must admit, I'm not fully convinced by this approach. What if we added a new method to all the DictList classes here called _repr_tr which returns a representation of itself wrapped in <tr></tr> so that it can be included easily? Guess we'd also need a class method _repr_th_ then for the column headers.

Of course, your approach has the benefit of easily transforming to a data frame which may have more general benefits.

Comment on lines +558 to +561

for item in self:
ids.append(item.id)
data.append([getattr(item, attr) for attr in columns])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for item in self:
ids.append(item.id)
data.append([getattr(item, attr) for attr in columns])
from operator import attrgetter
column_getter = attrgetter(*columns)
for item in self:
ids.append(item.id)
data.append(column_getter(item))

item = None
columns = []

if self:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's clearer to say

Suggested change
if self:
if not self.empty():

@cdiener cdiener added the stale The issue or pull request lacks activity. label Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale The issue or pull request lacks activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Neater notebook representation of Model Objects
4 participants