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

frozendict repr error w/ Metabolite entries #502

Closed
pstjohn opened this Issue May 4, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@pstjohn
Contributor

pstjohn commented May 4, 2017

In dubugging reaction.py (#503), I got an error when I tried to print out old_metabolites, a frozendict. Error message blow, looks like this was introduced with the new YAML round-tripping (#496). Pinging @Midnighter, do we need the sort to make the yaml output deterministic? Should we have a separate FrozenOrderedDict vs. FrozenDict? (why do we have frozendicts? 😄 )

In [3]: old_coefficients
Out[3]: ---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/Users/pstjohn/anaconda3/envs/cobra/lib/python3.6/site-packages/IPython/core/formatters.py in __call__(self, obj)
    670                 type_pprinters=self.type_printers,
    671                 deferred_pprinters=self.deferred_printers)
--> 672             printer.pretty(obj)
    673             printer.flush()
    674             return stream.getvalue()

/Users/pstjohn/anaconda3/envs/cobra/lib/python3.6/site-packages/IPython/lib/pretty.py in pretty(self, obj)
    381                             if callable(meth):
    382                                 return meth(obj, self, cycle)
--> 383             return _default_pprint(obj, self, cycle)
    384         finally:
    385             self.end_group()

/Users/pstjohn/anaconda3/envs/cobra/lib/python3.6/site-packages/IPython/lib/pretty.py in _default_pprint(obj, p, cycle)
    501     if _safe_getattr(klass, '__repr__', None) not in _baseclass_reprs:
    502         # A user-provided repr. Find newlines and replace them with p.break_()
--> 503         _repr_pprint(obj, p, cycle)
    504         return
    505     p.begin_group(1, '<')

/Users/pstjohn/anaconda3/envs/cobra/lib/python3.6/site-packages/IPython/lib/pretty.py in _repr_pprint(obj, p, cycle)
    699     """A pprint that just redirects to the normal repr function."""
    700     # Find newlines and replace them with p.break_()
--> 701     output = repr(obj)
    702     for idx,output_line in enumerate(output.splitlines()):
    703         if idx:

/Users/pstjohn/Packages/cobrapy/cobra/util/util.py in __repr__(self)
     36     def __repr__(self):
     37         if self.__items is None:
---> 38             self.__items = sorted(iteritems(self.__dict))
     39         return '{}({!r})'.format(self.__class__.__name__, self.__items)
     40

TypeError: '<' not supported between instances of 'Metabolite' and 'Metabolite'

@pstjohn pstjohn assigned pstjohn and unassigned pstjohn May 5, 2017

@Midnighter

This comment has been minimized.

Show comment
Hide comment
@Midnighter

Midnighter May 5, 2017

Member

Yes, my mistake. (Or lack of tests 😛 ) I changed FrozenDict. I've seen it in one place only so far which is to prevent direct changes when you get the Reaction.metabolites.

Member

Midnighter commented May 5, 2017

Yes, my mistake. (Or lack of tests 😛 ) I changed FrozenDict. I've seen it in one place only so far which is to prevent direct changes when you get the Reaction.metabolites.

@Midnighter

This comment has been minimized.

Show comment
Hide comment
@Midnighter

Midnighter May 5, 2017

Member

I suppose a quick solution would be to scrap FrozenDict and simply return a shallow copy.

Member

Midnighter commented May 5, 2017

I suppose a quick solution would be to scrap FrozenDict and simply return a shallow copy.

@pstjohn

This comment has been minimized.

Show comment
Hide comment
@pstjohn

pstjohn May 5, 2017

Contributor

Seems like a lot of code just for that reason, I'd be happy with a copy. Wouldn't be any slower than the frozendict -- looks like there was a PEP for unmutable dicts that got rejected.

Would we then need to use OrderedDicts in the yaml export?

Contributor

pstjohn commented May 5, 2017

Seems like a lot of code just for that reason, I'd be happy with a copy. Wouldn't be any slower than the frozendict -- looks like there was a PEP for unmutable dicts that got rejected.

Would we then need to use OrderedDicts in the yaml export?

@Midnighter

This comment has been minimized.

Show comment
Hide comment
@Midnighter

Midnighter May 6, 2017

Member

Would we then need to use OrderedDicts in the yaml export?

Actually, I used OrderedDict in cobra.io.dict a lot and Reaction.metabolites is the only part that I had to sort by ID for consistent results.

Member

Midnighter commented May 6, 2017

Would we then need to use OrderedDicts in the yaml export?

Actually, I used OrderedDict in cobra.io.dict a lot and Reaction.metabolites is the only part that I had to sort by ID for consistent results.

@Midnighter Midnighter self-assigned this May 6, 2017

@Midnighter Midnighter added the ready label May 6, 2017

@hredestig

This comment has been minimized.

Show comment
Hide comment
@hredestig

hredestig May 7, 2017

Contributor

fixed by #504

Contributor

hredestig commented May 7, 2017

fixed by #504

@hredestig hredestig closed this May 7, 2017

@hredestig hredestig removed the ready label May 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment