[BUG] Setting model.compartments asymmetric with getting

[oxinabox]

Is there an existing issue for this?

• I have searched the existing issues

Problem description

I am programatically manipulating some models
I thought I would set model.compartments using a dict,
then, as part of adding metabolites to it I would iterate the list of compartments.
(Basically for these metabolites i want to add one copy to each compartment).

However, when reading model.compartments it only returns compartments that have at least one metabolite in them already.

So you can set it, and it looks like nothing happens

Code sample

Code run:

In [83]: m = cobra.Model()

In [84]: m.compartments = {'a': "first compartment", 'b': "second compartment"}

In [85]: m.compartments
Out[85]: {}

In [86]: m.add_metabolites(cobra.Metabolite("X", "mystery", compartment='b'))

In [87]: m.compartments
Out[87]: {'b': 'second compartment'}

Environment

idk
but you can see the problem in the code here:

cobrapy/src/cobra/core/model.py

Lines 224 to 261 in 0e99b1d

	@property
	def compartments(self) -> Dict:
	"""Return all metabolites' compartments.
	Returns
	-------
	dict
	A dictionary of metabolite compartments, where the keys are the short
	version (one letter version) of the compartments, and the values are the
	full names (if they exist).
	"""
	return {
	met.compartment: self._compartments.get(met.compartment, "")
	for met in self.metabolites
	if met.compartment is not None
	}
	@compartments.setter
	def compartments(self, value: Dict) -> None:
	"""Get or set the dictionary of current compartment descriptions.
	Assigning a dictionary to this property updates the model's
	dictionary of compartment descriptions with the new values.
	Parameters
	----------
	value : dict
	Dictionary mapping compartments abbreviations to full names.
	Examples
	--------
	>>> from cobra.io import load_model
	>>> model = load_model("textbook")
	>>> model.compartments = {'c': 'the cytosol'}
	>>> model.compartments
	{'c': 'the cytosol', 'e': 'extracellular'}
	"""
	self._compartments.update(value)

Anything else?

I think probably the most sensible thing would be to deprecate the setter for Model.compartments,
and replace it with a function update_compartments or add_compartments.
and maybe replace the read property with a occupied_compartments property?

[cdiener]

Sorry about that. Compartments were supposed to become objects at some point for better compatibility with SBML but this was abandoned half-way, that's why we have the odd Frankenstein implementation. There is a "single source of truth" implementation for most things in cobrapy so that you can't have conflicting information from two places (because that was the source of many bugs historically). You can specify compartments by assigning them to metabolites or you could set them directly on the Model. To have those not conflict the only source for compartments are the Metabolites so they are always only inferred from there. But at some point we also wanted descriptions/names so there is now a dictionary stored in the Model class that keeps those:

In [6]: m = cobra.Model()
   ...: 
   ...: m.compartments = {'a': "first compartment", 'b': "second compartment"}
   ...: 
   ...: print(m.compartments)
   ...: 
   ...: print(m._compartments)
   ...: 
   ...: m.add_metabolites(cobra.Metabolite("X", "mystery", compartment='b'))
   ...: 
   ...: m.compartments
{}
{'a': 'first compartment', 'b': 'second compartment'}
Out[6]: {'b': 'second compartment'}

So it should probably be something along the lines of compartment_annotations or something. A modifier to return all declared compartments or only occupied ones also makes sense to me.