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

prepHumanModelForftINIT should take the paths to reactions.tsv and essential tasks as optional params #401

Closed
2 of 3 tasks
johan-gson opened this issue Jul 11, 2022 · 3 comments
Assignees

Comments

@johan-gson
Copy link
Collaborator

Description of the issue:

The current code is problematic since it uses "which('reactions.tsv')" - if we use for example the mouse model, it will grab the file for Human-GEM and so forth. I'm making an issue so we don't forget to change this.

Expected feature/value/output:

Just add these as params.

Current feature/value/output:

Reproducing these results:

I hereby confirm that I have:

  • Tested my code on my own computer for running the model
  • Done this analysis in the main branch of the repository
  • Checked that a similar issue does not exist already
@johan-gson johan-gson self-assigned this Jul 11, 2022
@JonathanRob
Copy link
Collaborator

JonathanRob commented Jul 12, 2022

Another option instead of making them into params is to code their paths relative to the location of the prepHumanModelForftINIT function. Since the location of the files relative to the function is not likely to change often or at all, I think it would be sufficiently stable.

For example, the translateGrRules function uses the following code to read the genes.tsv file whose relative path from the function is ../../model/genes.tsv:

[ST, I] = dbstack('-completenames');
path = fileparts(ST(I).file);
tmpfile = fullfile(path,'..','..','model','genes.tsv');

@johan-gson
Copy link
Collaborator Author

This would be an improvement to the default option, agreed, good suggestion! I think however that it would still be problematic for the mouse model. So, when using the mouse model, we would still use the function in Human-GEM, but with different files and a different model. The automatic code will then take the reactions.tsv from Human-GEM. In practice, reactions.tsv is most likely the same, but it still doesn't feel right to use the one in Human-GEM. I propose then to use the code you proposed above as default option, but still make possible to send the path in as a parameter for the two files. In practice, this will not matter much, but maybe they will diverge at some point.

@mihai-sysbio mihai-sysbio changed the title prepHumanModelForftINIT should take the pathways to reactions.tsv and essential tasks as optional params prepHumanModelForftINIT should take the paths to reactions.tsv and essential tasks as optional params Jul 28, 2022
@johan-gson
Copy link
Collaborator Author

I think we can close this now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants