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

Update calculate_stability usage #871

Merged

Conversation

sjspielman
Copy link
Member

Closes #870
Pretty much does what it says, following AlexsLemonade/rOpenScPCA#4!
Files were styled by pre-commit, too.

@sjspielman sjspielman marked this pull request as draft November 8, 2024 20:49
@sjspielman sjspielman removed the request for review from jaclyn-taroni November 8, 2024 20:49
@sjspielman sjspielman marked this pull request as ready for review November 8, 2024 20:55
@sjspielman sjspielman marked this pull request as draft November 8, 2024 20:58
@sjspielman
Copy link
Member Author

I did retrigger this run after the rOpenScPCA update went in, but maybe the environment was cached.

@sjspielman
Copy link
Member Author

Environment indeed seems to be cached. Let's just re-trigger after the weekend!

@sjspielman
Copy link
Member Author

sjspielman commented Nov 11, 2024

Still mostly cached, but there was this line which makes me think rOpenScPCA was not cached, in which case I do not really understand why the updated rOpenScPCA wasn't retrieved. @jashapiro, do you have any ideas?

Installing rOpenScPCA ...      OK [built from source and cached in 20s]

@jashapiro
Copy link
Member

Still mostly cached, but there was this line which makes me think rOpenScPCA was not cached, in which case I do not really understand why the updated rOpenScPCA wasn't retrieved. @jashapiro, do you have any ideas?


Installing rOpenScPCA ...      OK [built from source and cached in 20s]

Did you update renv.lock? It does record the commit hash, so I would expect that it is using that to retrieve the correct version.

@sjspielman
Copy link
Member Author

Did you update renv.lock? It does record the commit hash, so I would expect that it is using that to retrieve the correct version.

Ah, I had assumed since we didn't do release/tag (though, we probably need to adopt a development branch over there...) that wouldn't change. Perhaps bad assumption..

@jashapiro
Copy link
Member

we probably need to adopt a development branch over there...

I don't think that we do, at least not at this time.

@sjspielman
Copy link
Member Author

When I snapshot'ed analyses/cell-type-ETP-ALL-03/renv.lock, a bunch of new packages got added. I guess it just had missed a snapshot in there at some point during development, but not enough to affect CI...?

@sjspielman sjspielman marked this pull request as ready for review November 12, 2024 15:20
@@ -2076,8 +2372,8 @@
"RemoteHost": "api.github.com",
"RemoteUsername": "AlexsLemonade",
"RemoteRepo": "rOpenScPCA",
"RemoteRef": "main",
"RemoteSha": "fc784446f8d86b072e6f7f67287adfff598f4911",
"RemoteRef": "f71a8191130fa543e6506c73c7b62ffa55e8ba3f",
Copy link
Member

Choose a reason for hiding this comment

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

Did you make this update manually, installing the commit directly? I would have expected the RemoteRef to stay as main, which will enable future updates with just renv::update("rOpenScPCA"). Otherwise we have to keep track of the hashes much more manually, which is annoying.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did renv::install("rOpenScPCA@......"). Will go again with update

Copy link
Member

Choose a reason for hiding this comment

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

You should be able to just change these lines manually.

Suggested change
"RemoteRef": "f71a8191130fa543e6506c73c7b62ffa55e8ba3f",
"RemoteRef": "main",

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

LGTM

@sjspielman sjspielman merged commit 2b824bf into AlexsLemonade:main Nov 13, 2024
10 checks passed
@sjspielman sjspielman deleted the sjspielman/870-update-stability-usage branch November 13, 2024 14:35
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 rOpenScPCA::calculate_stability usage
2 participants