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: python I/O functions + BiGG compliance flag #224

Merged
merged 6 commits into from
Jun 10, 2020

Conversation

BenjaSanchez
Copy link
Contributor

Main improvements in this PR:

  • Features python functions for loading and saving the model (read_yeast_model and write_yeast_model) without needing to specify the model path. Usage, as indicated now in the README file, is as follows:
    import ComplementaryScripts.io as io
    model = io.read_yeast_model() # loading
    io.write_yeast_model(model)   # saving
    For this to work, a .env is required in the repo's root (as also indicated in the README file).
  • Features a requirements.txt file for ensuring a reproducible environment when performing simulations in Python.
  • To fully address compatibility of identifiers with BIGG database #172, read_yeast_model features a make_bigg_compliant flag for loading the model with BiGG ids instead of regular ids. If chosen, the BiGG annotation in the model + the BiGG dictionaries in ./ComplementaryData/databases are used for the conversion. Some additional info:
    • ids will have a _copyN suffix if they are repeated in the model (as ids should be unique).
    • ids not found will be kept the same as in the model (but in the case of metabolites, converted to the met_comp standard).
    • Original ids are moved to notes.
    • Compartments are made BiGG-compliant with the dict {"er":"r", "erm":"rm", "p":"x"}
    • Compartment info is removed from metabolite names.

NOTE that in each commit I saved the model with write_yeast_model to confirm changes were as expected, but in the last commit I reverted the model to be as the original one in devel, so no model changes are performed here.

NOTE ALSO that although the model can be saved in cobrapy format keeping all fields, it changes considerably the structure of the .xml file and hence should not be done if contributing to this repo until issue #218 is solved.

I hereby confirm that I have:

  • Tested my code with all requirements for running the model
  • Selected devel as a target branch (top left drop-down menu)

Added functions for loading/saving the model in python in a more straightforward way. Also the id replacement is no longer necessary, as it was fixed in cobrapy. Updated the model as a test (will be reverted later)
minor changes, including shuffling of groups (as the latest cobrapy version still treats them as sets)
add a BiGG compliance flag that defaults to false when loading the model. If true, will read the annotation and use BiGG ids (if any)
model cannot be stored at this stage, as there are repeated ids in the BiGG dicts
introduce a copy suffix when id is repeated
@BenjaSanchez BenjaSanchez added the enhancement new field/feature label May 28, 2020
@BenjaSanchez BenjaSanchez requested a review from hongzhonglu May 28, 2020 10:35
@BenjaSanchez BenjaSanchez self-assigned this May 28, 2020
@BenjaSanchez BenjaSanchez requested a review from edkerk May 28, 2020 10:36
@BenjaSanchez BenjaSanchez changed the title feat: better python I/O functions + flag for BiGG compliance feat: python I/O functions + BiGG compliance flag May 28, 2020
@hongzhonglu
Copy link
Collaborator

hongzhonglu commented May 28, 2020

Hi @BenjaSanchez I am not sure whether errors exist when load model using python, i have used the followed self-function to correct some errors:
def correctSomeWrongFormat(model0):
"""
This function is used to correct some wrong format when read yeastGEM model from cobratoolbox
"""
for met in model0.metabolites:
met.id = met.id.replace('91', '_')
met.id = met.id.replace('93', '')
#for reaction in model0.reactions:
for gene in model0.genes:
gene.id = gene.id.replace('45', '-')

return model0

Copy link
Member

@edkerk edkerk left a comment

Choose a reason for hiding this comment

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

I have no experience with running cobrapy, so I'm unable to review this functionality.

@BenjaSanchez
Copy link
Contributor Author

@hongzhonglu the changes you are performing are not needed anymore with the latest versions of cobrapy, as they convert the non-ASCII characters by default. Are you testing the PR with the proper environment? please update your cobrapy version, or even better, pip install -r requirements.txt in a fresh environment to make sure we have the same setup :)

@hongzhonglu
Copy link
Collaborator

Hi @BenjaSanchez, Thanks for your information. I will test it.

@BenjaSanchez
Copy link
Contributor Author

BenjaSanchez commented Jun 8, 2020

@hongzhonglu is the setup working now for you? :)

@hongzhonglu
Copy link
Collaborator

hi Ben, could you replace this package named "dotenv" as i found it is can't be installed by pycharm?

@BenjaSanchez
Copy link
Contributor Author

@hongzhonglu the package is called python-dotenv, as you can see in requirements.txt (no package called dotenv). Pycharm should not have any problem with python-dotenv.

@hongzhonglu
Copy link
Collaborator

hongzhonglu commented Jun 8, 2020

Hi Ben, thanks for your remind!
I meet new issue for the two steps:
dotenv_path = find_dotenv()
REPO_PATH = dirname(dotenv_path)

when run the above code, REPO_PATH is "", thus the followed commond can not be run. Why not directly set the whole path of model as the input for function "read_yeast_model"?

@BenjaSanchez
Copy link
Contributor Author

@hongzhonglu did you follow the new installation instructions as the README now states? You should create an empty .env file in the repo root:

type nul > .env                     # creates a .env file for locating the root

The idea of using dotenv is precisely to avoid specifying the path depending of where in the repo we are, as that (absolute) path never changes. This makes it easier & safer for the user to read/write, as it requires less typing and avoids mistaking the path and saving the model somewhere undesired.

@hongzhonglu
Copy link
Collaborator

Now, i can run it!

@BenjaSanchez BenjaSanchez merged commit 984dca1 into devel Jun 10, 2020
@BenjaSanchez BenjaSanchez deleted the feat/python-compatibility branch June 10, 2020 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new field/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants