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

Human 1.19 deprecated since this does not follow the guidelines #819

Closed
wants to merge 149 commits into from

Conversation

JHL-452b
Copy link
Collaborator

@JHL-452b JHL-452b commented May 8, 2024

Main improvements in this PR:

I hereby confirm that I have:

  • Tested my code on my own computer for running the model
  • Selected develop as a target branch
  • Any removed reactions and metabolites have been moved to the corresponding deprecated identifier lists

johan-gson and others added 30 commits May 21, 2023 16:49
…ion. I don't think that code had ever been run, it was just not right.
…urrently only processes the model available in the repo, and does not compare with any older model version. The code is however prepared for comparing versions. Currently, it only processes 2 out of 40 chunks of data, this is to be changed when we want to run a full evaluation with 891 cell lines.
Added metabolite SMILES and Inchi Information
…d 2-OADH reactions and thiamine-to-lipoyl transfer for 2-OADH
feiranl and others added 19 commits March 19, 2024 10:42
fix: standardize all names of exchange reactions
feat: add gene essentiality check using DepMap
Resolve Duplicate Phytanoyl-CoA Alpha-Oxidation Reactions
…linolenoyl_beta_ox_paths

Merge Duplicate Mitochondrial Linoleic and Gamma-Linolenic Acid Beta-Oxidation Pathways
…to_gprs

fix: GPRs for mitochondrial 2-enoyl-CoA hydratase reactions
Fix Stereochemistry of Phytanic Acid Oxidation
…actions

fix: modified unbalance reactions
Fix Mitochondrial Thiamine Transport Reactions
@JHL-452b JHL-452b changed the title Develop Human 1.19 May 8, 2024
@feiranl feiranl marked this pull request as draft May 8, 2024 14:48
@mihai-sysbio
Copy link
Member

Looking quickly in the changed files, most seem okay. However, I can already flag some changes that have to be verified one by one: there are lines deleted from the deprecated files. This would mean that an ID was deprecated, in a previous release, and either un-deprecated now or an accident. Ideally this should never happen - deprecation should be a one-way street. So, for all the reversals of the deprecation, it should be checked that those make sense.

@feiranl you mentioned conflicts betwen feature branches. Were you able to track back to the original pull requests at the source of the confict?

Another thing, could an updated gene essentiality table be provided, as in the PR for the previous release?

@feiranl
Copy link
Collaborator

feiranl commented May 9, 2024

Some IDs were recovered from the deprecated files. I looked into the conflict in yaml file, it should be these reactions:MAR08415;MAR03984;MAR12372;MAR20021;MAR20022;MAR20023;MAR20024;MAR20025;MAR20026;MAR20027;MAR20028;MAR20029;MAR20030, because we changed twice. The final format for those reaction should be as in the attached file.Human_1.19_conflict.txt
@JHL-452b Could you list those PRs for these conflicts?

@mihai-sysbio I need to mention that this PR is not done following the guidelines of making a release, rather done like merging from feature branch to devel branch, will that be an issue? If so, we can igonre this and redo it following the guidelines.

@JHL-452b
Copy link
Collaborator Author

JHL-452b commented May 9, 2024

Some IDs were recovered from the deprecated files. I looked into the conflict in yaml file, it should be these reactions:MAR08415;MAR03984;MAR12372;MAR20021;MAR20022;MAR20023;MAR20024;MAR20025;MAR20026;MAR20027;MAR20028;MAR20029;MAR20030, because we changed twice. The final format for those reaction should be as in the attached file.Human_1.19_conflict.txt @JHL-452b Could you list those PRs for these conflicts?

Of course!
Reactions: MAR08415;MAR03984;MAR12372 were modified twice in PR #670 and PR #816.
Reactions: MAR20021;MAR20022;MAR20023;MAR20024;MAR20025;MAR20026;MAR20027;MAR20028;MAR20029;MAR20030 were modified twice in PR #712 and PR #816.

@mihai-sysbio
Copy link
Member

@mihai-sysbio I need to mention that this PR is not done following the guidelines of making a release, rather done like merging from feature branch to devel branch, will that be an issue? If so, we can igonre this and redo it following the guidelines.

@feiranl the steps in the release guidelines have to be followed otherwise there will be consequences downstream, such as incorrect versions listed (hinted in the last bullet point).

@feiranl
Copy link
Collaborator

feiranl commented May 28, 2024

@mihai-sysbio Thanks! Totally understand. We are planning a new PR following the guidelines. This will be deleted later. But we need to wait until the RAVEN function is merged and the check is back to normal

@edkerk May I know if there is any plan of when it will be merged?

@edkerk
Copy link
Member

edkerk commented May 28, 2024

@feiranl RAVEN 2.9.2 was released yesterday

@feiranl
Copy link
Collaborator

feiranl commented May 28, 2024

Nice! thanks Ed. Will close this PR and reopen a new one to release the Human 1.19 @JHL-452b

@feiranl feiranl closed this May 28, 2024
@feiranl feiranl changed the title Human 1.19 Human 1.19 deprecated since this does not follow the guidelines May 28, 2024
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

Successfully merging this pull request may close these issues.

7 participants