#652[Feature request] Provide compartments as objects #696

MaxGreil · 2018-04-18T18:28:46Z

#652
Implemented model.compartments as objects, therefore added new class compartment in cobra.core.

In cobra.core: modified class cobra.model to store new class compartment correctly.
In cobra.io: modified json.py, dict.py, mat.py, sbml.py and sbml3.py.
In cobra.manipulation: modified modify.py.
In cobra.test.data: run update_pickles.py and modified existing data files according to new class compartment.
In cobra.test: modified tests according to new class compartment.

EDIT 20.04.:

In cobra.util: modified version_info.py by importing pkg_resources from setuptools for function pkg_resources.working_set. Dropped pip.get_installed_distributions().

EDIT 21.04.:

In cobra.test: modified test_io.py to be compartible for pickles from both python2 and python3

codecov-io · 2018-04-20T16:50:58Z

Codecov Report

Merging #696 into devel will increase coverage by 0.04%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##            devel     #696      +/-   ##
==========================================
+ Coverage   81.72%   81.76%   +0.04%     
==========================================
  Files          36       37       +1     
  Lines        3644     3691      +47     
  Branches      832      843      +11     
==========================================
+ Hits         2978     3018      +40     
- Misses        472      477       +5     
- Partials      194      196       +2

Impacted Files	Coverage Δ
cobra/manipulation/modify.py	`92.39% <ø> (ø)`	⬆️
cobra/util/version_info.py	`100% <ø> (ø)`	⬆️
cobra/io/json.py	`60% <ø> (ø)`	⬆️
cobra/io/mat.py	`68.2% <100%> (ø)`	⬆️
cobra/io/sbml3.py	`82.08% <100%> (ø)`	⬆️
cobra/io/sbml.py	`65.77% <60%> (ø)`	⬆️
cobra/core/compartment.py	`71.42% <71.42%> (ø)`
cobra/core/model.py	`90.09% <84.61%> (-0.46%)`	⬇️
cobra/io/dict.py	`85.12% <94.73%> (+1.47%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e897956...20833b9. Read the comment docs.

MaxGreil · 2018-04-21T16:59:59Z

All TravisCI tests are now working.
The new funcionality of the PR is as requested in #652 . Compartments are now not just a dictionary but objects like all the other model components. They can be accessed like all other model components with model.compartments which returns the DictList containing all compartments of the model.
New class compartment inherits from class object and therefore possesses the same functionality like all other model components.
Code reviewer: @cdiener

Midnighter

One concern about the files changed for the tests. Did you intentionally and (more or less) manually change the mini_cobra.xml and mini_fbc*.xml? And why did the textbook* solutions change? If this all happened due to the update script then I would ask you please to checkout the previous versions of the solutions. Simply to ensure that there are no changes creeping in over time.

I want to look at the compartment class a little more but it looks like great work!

MaxGreil · 2018-04-26T16:39:53Z

@Midnighter I only used the update_pickles.py script.
As far as I can tell, the files like mini_cobra.xml and mini_fbc*.xml were shifted around a bit but this did not change the context/variables/values.
The same goes for the textbook* solutions which have no changed values but were also shifted around a bit.

cdiener

model.comaprtments should assemble a list of compartments dynamically from the metabolites in the model like before. For instance the following is now working incorrectly:

In [2]: from cobra.test import create_test_model

In [3]: mod = create_test_model("textbook")

In [5]: mod.compartments
Out[5]: [<Compartment c at 0x7f4d164bf048>, <Compartment e at 0x7f4d164bf0b8>]

In [6]: mod.metabolites.ac_e.compartment = "m"

In [7]: mod.metabolites.ac_e.compartment
Out[7]: 'm'

In [8]: mod.compartments
Out[8]: [<Compartment c at 0x7f4d164bf048>, <Compartment e at 0x7f4d164bf0b8>]

This should include a compartment "m" with an empty name.

Also we will need to maintain backwards compatibility so compartments should be optional in JSON and YAML.

cdiener · 2018-04-27T21:04:36Z

cobra/core/model.py

@@ -161,32 +166,6 @@ def get_metabolite_compartments(self):
        return {met.compartment for met in self.metabolites
                if met.compartment is not None}

-    @property
-    def compartments(self):


I think this should not be removed see general comment.

cdiener · 2018-04-27T21:05:49Z

cobra/core/model.py

+        compartment_list = [x for x in compartment_list
+                            if x.id not in self.compartments]
+
+        bad_ids = [m for m in compartment_list


spaces should probably be disallowed as well since we will get problems with optlang otherwise.

cdiener · 2018-04-27T21:08:07Z

cobra/io/json.py

        },
        "notes": {"type": "object"},
        "annotation": {"type": "object"},
    },
-    "required": ["id", "reactions", "metabolites", "genes"],
+    "required": ["id", "reactions", "metabolites", "genes", "compartments"],


We need to maintain backwards compatibility so compartments should be optional for all input formats.

cdiener · 2018-04-27T21:10:03Z

cobra/test/data/mini.json

-        "c": "cytosol",
-        "e": "extracellular"
-    },
+    "compartments": [


Should also include a version ("mini_legacy.json") without those to test backwards compatibility.

cdiener · 2018-04-27T21:11:11Z

cobra/test/data/mini.yml

@@ -1143,6 +1143,10 @@
    - name: G_s0001
 - id: mini_textbook
 - compartments:
-    e: extracellular
-    c: cytosol
+    - !!omap


That looks a bit weird. What was wrong with the old notation?

The ordered map is introduced by the YAML IO automatically but I agree, let's keep the old format of id: name.

cdiener · 2018-04-27T21:13:15Z

cobra/test/test_io.py

+        try:
+            reference_model = load(infile)
+        except UnicodeDecodeError:
+            reference_model = load(infile, encoding='latin1')


Why that? I think we should not include any files with latin-1. Everything should be unicode.

I agree, please store files in utf-8 format.

cdiener · 2018-04-27T21:13:52Z

cobra/test/test_io.py

@@ -182,7 +185,7 @@ def compare_models(cls, name, model1, model2):

    @classmethod
    def extra_comparisons(cls, name, model1, model2):
-        assert model1.compartments == model2.compartments
+        assert len(model1.compartments) == len(model2.compartments)


Should also check if they contain the same IDs.

cdiener · 2018-04-27T21:14:27Z

cobra/util/version_info.py

@@ -12,6 +12,8 @@

 import pkg_resources

+import pkg_resources


Thanks for that trick!

Please revert the changes to this file since the issue was addressed in #697.

cdiener · 2018-04-27T21:17:16Z

cobra/core/compartment.py

+        """
+    def __init__(self, id=None, name=""):
+        Object.__init__(self, id, name)
+        self._model = None


I don't think it needs a back-ref to the model. That just creates problems.

cdiener · 2018-04-27T21:18:27Z

Thanks for all the work! Sorry that there are so many comments but for new features we have to ensure that nothing breaks. Cheers!

…rtments

MaxGreil · 2018-04-29T14:17:15Z

Compartments of cobra.model are now a property.
In file test_io.py in case of a UnicodeDecodeError first an encoding with utf-8 is tried and if this fails an encoding with latin1 is tried. This is a work-around when loading python2 pickles in python3. See https://stackoverflow.com/questions/28218466/unpickling-a-python-2-object-with-python-3.

cdiener · 2018-05-29T19:55:47Z

Appveyor seems to work again.

Midnighter · 2018-06-13T18:32:37Z

Dear @MaxGreil, I'm very sorry to be doing this but we need to move things along right now. Thus I took your branch, rebased it on the current devel and will take over development on that new branch. Thank you very much for contribution and I hope this is not offending you. Please see #725 for the new PR.

MaxGreil · 2018-06-14T17:35:03Z

Hi @Midnighter, I didn't have the time to continue working on this issue.

Midnighter · 2018-06-14T17:36:40Z

That's completely understandable. Would you like to finish it up, though?

MaxGreil · 2018-06-17T06:33:46Z

I would still like to. What is left to be done?

s9magrei and others added 11 commits April 12, 2018 20:30

Added class compartment

274a28b

Modified cobra.model for cobra.model.compartments

73cfd2c

Modified cobra.model add_metabolites function for compartments

3bff4bb

Modified json and yml for new class model.compartments

1b0b91d

Modified sbml and sbml3 plus test_model for new model.compartments

3fbb27c

Refreshed files with update_pickels, modified model, sbml3, mat

ecbd37c

fixed some errors for new class compartment in tests

8584119

fixed mini.json

818abfe

fixed pep8

324ffb0

fixed errors in test_util.py

f3900b1

fixed errors in test_io.py

f9d2b61

MaxGreil added 3 commits April 21, 2018 14:01

Test fix for test_io.py

1123475

Next try

8e20e26

Test with latin1

0da69be

cdiener self-requested a review April 23, 2018 14:40

cdiener mentioned this pull request Apr 24, 2018

Repair Appveyor #697

Merged

Merge branch 'devel' into Compartments

8ec223c

Midnighter requested changes Apr 25, 2018

View reviewed changes

cdiener requested changes Apr 27, 2018

View reviewed changes

MaxGreil added 5 commits April 28, 2018 15:05

some of the requested changes

ae97bf9

deleted files

a2e8b1e

utf-8 encoding

cd35d1d

Merge branch 'devel' of git://github.com/opencobra/cobrapy into Compa…

cdb5afe

…rtments

Property compartments

989ba5b

Setter for compartments

9dba00e

Run update.py for new property compartments

20833b9

Midnighter closed this Jun 13, 2018

Midnighter mentioned this pull request Sep 6, 2018

refactor: turn compartments into objects #725

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#652[Feature request] Provide compartments as objects #696

#652[Feature request] Provide compartments as objects #696

MaxGreil commented Apr 18, 2018 •

edited

Loading

codecov-io commented Apr 20, 2018 •

edited

Loading

MaxGreil commented Apr 21, 2018 •

edited

Loading

Midnighter left a comment

MaxGreil commented Apr 26, 2018 •

edited

Loading

cdiener left a comment •

edited

Loading

cdiener Apr 27, 2018

cdiener Apr 27, 2018

cdiener Apr 27, 2018

cdiener Apr 27, 2018

cdiener Apr 27, 2018

Midnighter Apr 27, 2018

cdiener Apr 27, 2018

Midnighter Apr 27, 2018

cdiener Apr 27, 2018

cdiener Apr 27, 2018

Midnighter Apr 27, 2018

cdiener Apr 27, 2018

cdiener commented Apr 27, 2018

MaxGreil commented Apr 29, 2018 •

edited

Loading

cdiener commented May 29, 2018

Midnighter commented Jun 13, 2018

MaxGreil commented Jun 14, 2018

Midnighter commented Jun 14, 2018

MaxGreil commented Jun 17, 2018

#652[Feature request] Provide compartments as objects #696

#652[Feature request] Provide compartments as objects #696

Conversation

MaxGreil commented Apr 18, 2018 • edited Loading

codecov-io commented Apr 20, 2018 • edited Loading

Codecov Report

MaxGreil commented Apr 21, 2018 • edited Loading

Midnighter left a comment

Choose a reason for hiding this comment

MaxGreil commented Apr 26, 2018 • edited Loading

cdiener left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdiener commented Apr 27, 2018

MaxGreil commented Apr 29, 2018 • edited Loading

cdiener commented May 29, 2018

Midnighter commented Jun 13, 2018

MaxGreil commented Jun 14, 2018

Midnighter commented Jun 14, 2018

MaxGreil commented Jun 17, 2018

MaxGreil commented Apr 18, 2018 •

edited

Loading

codecov-io commented Apr 20, 2018 •

edited

Loading

MaxGreil commented Apr 21, 2018 •

edited

Loading

MaxGreil commented Apr 26, 2018 •

edited

Loading

cdiener left a comment •

edited

Loading

MaxGreil commented Apr 29, 2018 •

edited

Loading