nadeemlab / SPT

Spatial profiling toolbox for spatial characterization of tumor immune microenvironment in multiplex images
https://oncopathtk.org
Other
21 stars 2 forks source link

Convert cggnn/graphs and workflow configure to use config files #281

Closed CarlinLiao closed 10 months ago

CarlinLiao commented 10 months ago

This closes #272 and #278 by

276 and spt-plugin#3 are necessary prerequisites for this PR as #276 is still dependent on an older version of spt-cg-gnn Docker image that will need to be updated once spt-plugin#3 is accepted. Until those are merged and that Docker image updated, this will remain an untested draft PR.

CarlinLiao commented 10 months ago

This was also accidentally merged by me at the same time as #276, but this wasn't a PR that was ready to merge. I think what happened is that I pressed the wrong button in the VS Code version control UI (but I didn't think that it was possible to merge pull requests in VS Code). Checking resolution methods...

jimmymathews commented 10 months ago

I think it's probably not a big deal, we can just reset the main branch pointer to the commit just before merge. No need to erase history, the branch containing unintentionally merged commits would just be anonymous/dangling.

CarlinLiao commented 10 months ago

I suspect but can't confirm what happened was that

  1. my local SPT repo was sitting in the issue272 branch
  2. I used the VS Code UI to switch to main branch and sync for new incoming commits and upload outgoing commits, but the switch only switched my branch reference and not repo state,
  3. so all the commits from issue272 got uploaded to main as if rebasing.

Consequently, there's no single merge commit to undo, all of the commits from issue275 and issue272 are in main as if I commit them to the main branch directly as I was working on them. Conveniently, however, the branches happened to be worked on serially over the last few weeks so it should be easy to split: every commit after this one is from issue275 until this one, after which it's commits for issue272.

CarlinLiao commented 10 months ago

I think the simplest solution may be to move main back to this commit..

git reset --hard 04e2fef
git push origin main -f

but I won't do this myself because I've messed up the SPT commit history enough today as is.

jimmymathews commented 10 months ago

Yes, that it what I was going to recommend (not sure about whether --hard is necessary though). I believe the issue branches can be worked on normally after this. I'll give it try.

jimmymathews commented 10 months ago

It looks OK to me.

CarlinLiao commented 10 months ago

Thank you for fixing my mistake, it looks good on my end as well. I don't see a method to re-open these pull requests, though I may not have the necessary permissions for that.

jimmymathews commented 10 months ago

Hm well the branches are still there (e.g. issue272) so I think you can just make a new PR.