search

MetabolicAtlas / data-files

Source data files used when deploying Metabolic Atlas
0 stars 0 forks source link

Feat: collect release data for GEMs repository #26

Closed nanjiangshu closed 2 years ago

nanjiangshu commented 2 years ago

This PR closes #954

Changes made

Additional notes externalParentId, count and validation are not added in the PR. If someone has a complete view of the externalParentId info, I can add it to this PR as well. Obtaining the values for the count and validation will be addressed in new issues.

MalinAhlberg commented 2 years ago

Very nice, good work! :rocket:

One minor thing: I think it should be mentioned in the docs that PyGithub is needed. I also wonder if it wouldn't be good to have the externalParentIds before we close this issue?

nanjiangshu commented 2 years ago

We've decided to collect also externalParentId in this PR according to this discussion. The PR is convert to Draft temporary.

nanjiangshu commented 2 years ago

Very nice, good work! 🚀

One minor thing: I think it should be mentioned in the docs that PyGithub is needed. Thanks for pointing this out. I've added it in the Doc.

I also wonder if it wouldn't be good to have the externalParentIds before we close this issue? Yes. the issue has been converted to Draft temporary and the issue description is updated to add this requirement.

inghylt commented 2 years ago

Looks very nice so far! I think we should specify which version of PyGithub is used, maybe it would make sense to add a requirements.txt

mihai-sysbio commented 2 years ago

In general, I'd like us to always value simplicity. In the case of this repository, I would find it simpler if all functionality were coded in the same programming language. And since the main Metabolic Atlas repo is JavaScript-only, I'm very much leaning to having this one JS only as well.

nanjiangshu commented 2 years ago

Looks very nice so far! I think we should specify which version of PyGithub is used, maybe it would make sense to add a requirements.txt

Despite different opinions from the team. I added a requirement file in the commit d3bfbce Some additional packages are included in this file and they are for other python script in the utils folder.

nanjiangshu commented 2 years ago

Good job @nanjiangshu! I tested running this script and used the output JSON files to generate the timeline chart and it seems to work great!

Besides the minor comment about the API token, I noticed that a few new versions were released yesterday. Maybe it would be nice to have them along with this PR. Let me know if you want help with any of these tasks.

Thanks for pointing this out. I've upgrade the JSON files with the latest model releases in the commit d74c588