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

functions in rba/estim seem outdated #7

Open
m-jahn opened this issue May 12, 2020 · 3 comments
Open

functions in rba/estim seem outdated #7

m-jahn opened this issue May 12, 2020 · 3 comments
Assignees

Comments

@m-jahn
Copy link

m-jahn commented May 12, 2020

I tried to perform the protein_per_compartment.py analysis and made a custom config file and tables with input data. However, already loading the script in python3 produces a lot of errors. I got many indentation errors but also syntax errors for functions that seem outdated in python3?
Also, in contrast to the main RBApy package these scripts seem not that well maintained/organised. For example, the configuration file needs to be in the same folder rba/estim/ as the RBApy source code. It would be better to keep input data and source code seaparated.

Maybe we can fix this somehow? I would help if I can.

@AnneGoelzer
Copy link
Collaborator

Hello michael. Thanks for pointing out the problem. We will try to fix the problem quickly.

@m-jahn
Copy link
Author

m-jahn commented May 28, 2020

I recently went through the procedure of k_app estimation using the functions in RBA estim and got it to work with some modifications. I just post here some suggestions what could be improved, from my experience.

  • generally I think it would be more consistent if the RBA estim functions are part of the RBApy package just as the other functions, so that they are imported when running import rba. Right now they are somewhat standalone.
  • In kapp.py, the unit of protein concentration is hard-coded as copy numbers (it calls get_mmol_gcdw(p[1], cdw)). However I supplied proteomics data in mmol/gDCW, so I added a function get_copies_gcdw(p[1], cdw) to utils.py that does the opposite conversion. I also added a flag conc_in_copies = False to kapp.cfg.
  • it would be good to have a choice for some of the hardcoded options. E.g. to only include cytosolic or all proteins in kapp estimation (right now the default is to include only cytosolic). I added a flag cytoplasmic_only = False in kapp.cfg.
  • Similarly I added flags for other result processing options like nonzero_only = True and remove_duplicates = True (added a line to kapp.py that removes R_ABC1_duplicate reactions).
  • I modified kapp.py so that the format of the exported kapp table is directly usable as input for RBApy: convert kapp unit from 1/s to 1/h, remove enzyme duplicates (I assumed they are not used by RBApy), reformat enzyme names to R_ABC1_enzyme, copy kapp to second column to have upper and lower kapp boundaries, save table without header in tab-separated format, not comma-separated.

@abulovic abulovic self-assigned this May 31, 2020
@abulovic
Copy link
Collaborator

Thank you for indicating the issues and for your helpful comments. I am working on an overhaul of this package.

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

3 participants