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: update models #32

Merged
merged 7 commits into from
Sep 27, 2022
Merged

Feat: update models #32

merged 7 commits into from
Sep 27, 2022

Conversation

nanjiangshu
Copy link
Contributor

@nanjiangshu nanjiangshu commented Sep 22, 2022

This PR together with MetabolicAtlas/MetabolicAtlas#1145 closes #1081

Changes been done

  1. Human-GEM is updated to version 1.12.0
  2. Yeast-GEM is updated to version 8.6.2

For Yeast-GEM

  1. Some mismatches in the subsystemSVG.tsv and the YAML file are fixed manually
  2. custom maps that are already included in the model file (v8.6.2) are removed from the file customSVG.tsv

Regarding the mismatch of Yeast-GEM version cased by the patch releases, that is, v8.6.2 is shown in the integrated models panel but v8.6.0 is shown in the history map, it is probably good to add an explanation somewhere. Since there is no such text at any place regarding the history map, I think it is better to address this problem in a new issue.

Details can be found in this comment

Additional info
I've found a problem some weird text shown in the model search result table when viewing on Safari, but not on Firefox nor Chrome. Please verify if it is the case for you as well. For me, the problem exist when building with all main branches.
Screenshot 2022-09-22 at 11 17 09

@MalinAhlberg
Copy link
Contributor

Nice!
It works well as far as I can tell. Regarding the bug when searching for dd2 in Rat-GEM, I can not reproduce it, but I do not have Safari.
A question: when comparing to https://metabolicatlas.org/gems/repository, many more models seems to have been updated (eg FruitFly from 1.1.0 to 1.2.0). When was this done?

Copy link
Contributor

@inghylt inghylt left a comment

Choose a reason for hiding this comment

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

Nice work! It would be nice to get another Mac users input (ping @e0 or @mihai-sysbio) to see if they have the problem with weird text when searching. If so I don't think we should merge this before it is fixed.

And regarding the mismatch of Yeast-GEM version, I think it makes sense to create a new issue for this

@e0
Copy link
Contributor

e0 commented Sep 26, 2022

Mac

I can confirm that this problem shows up in Safari here too. Will try to go through the PR now to see if I can spot anything.

@e0
Copy link
Contributor

e0 commented Sep 26, 2022

I looked up this Safari problem a bit now. It seems like a Vite related problem that's also reported here: vuejs/vitepress#218

It seems like it's only affecting development mode so we should be fine. I can also confirm that if you manually set the encoding (UTF-8 instead of Default), these weird characters will disappear.

image

@e0
Copy link
Contributor

e0 commented Sep 26, 2022

A question: when comparing to https://metabolicatlas.org/gems/repository, many more models seems to have been updated (eg FruitFly from 1.1.0 to 1.2.0). When was this done?

It seems like most of these were done in this PR. It was merged at the end of May though so I'm a bit confused why the production site doesn't have these versions since we have deployed to it after that.

Copy link
Contributor

@e0 e0 left a comment

Choose a reason for hiding this comment

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

Great work @nanjiangshu!

@nanjiangshu
Copy link
Contributor Author

Nice! It works well as far as I can tell. Regarding the bug when searching for dd2 in Rat-GEM, I can not reproduce it, but I do not have Safari. A question: when comparing to https://metabolicatlas.org/gems/repository, many more models seems to have been updated (eg FruitFly from 1.1.0 to 1.2.0). When was this done?

As @e0 pointed out, most of the models are updated in #29 already in May. I guess the reason why old models are still used on the production server https://metabolicatlas.org/ is that the repo data-files were not up-to-date or in a different branch than main at the deployment.

@nanjiangshu
Copy link
Contributor Author

Many thanks to @e0 to figure out the problem with weird characters and good to know that it affects (hopefully) only the development mode.

Copy link
Contributor

@MalinAhlberg MalinAhlberg left a comment

Choose a reason for hiding this comment

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

👍

@nanjiangshu
Copy link
Contributor Author

@inghylt Do you have any other comments to this PR?

Copy link
Contributor

@inghylt inghylt left a comment

Choose a reason for hiding this comment

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

Nice work!

@nanjiangshu nanjiangshu merged commit 204b4fe into main Sep 27, 2022
@nanjiangshu nanjiangshu deleted the feat/update-models branch September 27, 2022 11:11
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.

update models
4 participants