Add function to perform graph-based clustering

[sjspielman]

Purpose/implementation Section

Please link to the GitHub issue that this pull request addresses.

Closes #745

What is the goal of this pull request?

This PR adds a function to perform clustering to the rOpenScPCA package. There are many file changes, because I've also merged in main to stay in sync, but review can focus only on code in packages/rOpenScPCA.

I'm filing this as a draft to get some initial feedback, and then can implement the rest (including adding more tests, which I know are thin right now!) and mark for review. Here are the main questions I'm looking for feedback on, but of other high-level feedback is welcome :)

• As a starting point (as in, we could do more/less!), I have included three arguments for specifying "additional" clustering parameters:resolution (used by louvain and leiden), objective_function (used by leiden), and cluster_args (for anything else). How does this choice feel to you? Prefer to see fewer or more of the algorithm-specific options pulled out as individual arguments?
  • Noting further I don't have any checks for these arguments as of yet, but sure could!
• Should the table the function returns contain additional columns, e.g. of other non-default parameters used for the clustering, beyond what I've coded up so far?
  • At one point I wondered if a DataFrame would be better since it store a lot of that info as metadata, but then I felt it might get confusing for users.
• Any thoughts on this comment? (noting that I edited the opening comment of Add rOpenScPCA function to perform graph-based clustering #745 after I wrote this; the matrix quoted there is the wrong matrix! we want pcs, not expression...)

Author checklists

Check all those that apply.
Note that you may find it easier to check off these items after the pull request is actually filed.

Analysis module and review

• This analysis module uses the analysis template and has the expected directory structure.
• The analysis module README.md has been updated to reflect code changes in this pull request.
• The analytical code is documented and contains comments.
• Any results and/or plots this code produces have been added to your S3 bucket for review.

Reproducibility checklist

• Code in this pull request has been added to the GitHub Action workflow that runs this module.
• The dependencies required to run the code in this pull request have been added to the analysis module Dockerfile.
• If applicable, the dependencies required to run the code in this pull request have been added to the analysis module conda environment.yml file.
• If applicable, R package dependencies required to run the code in this pull request have been added to the analysis module renv.lock file.

[jashapiro]

Looks good overall, but maybe file a separate PR with just the main changes to make this one smaller and easier to review?

I had a number of small suggestions for now, but overall this looks on the right track to me.

[jashapiro]

Yes, it probably should, to allow easier bind_rows to tidy form later. And not just non-default, but all args.

[sjspielman]

Or we can just check that all the args have length 1, and not support args that don't follow that pattern, as I don't think we expect to use anything like that...

I haven't really been able to think of anything better than this... I guess we could also return the cluster_args list for users to do what they want with, but given that a minority of users (if any) are likely to be in this circumstance, I do think just the straight data frame return is cleaner.

[sjspielman]

Ready for another look, and in particular interested in your feedback on my docs wording about parameter defaults, and overall wording for cluster_args length-1 restriction.

[jashapiro]

I was actually suggesting something a bit different for the docs; consolidating the discussion of differences into the description. I wrote a bit of what I was thinking there, but you will probably want to rewrite it.

[jashapiro]

I would actually leave off the explanation here. Instead, I was suggesting a paragraph of explanation before the @params, which will end up in the function description.

[sjspielman]

I wrote a bit of what I was thinking there, but you will probably want to rewrite it.

I think what you had there was mostly there tbh, so I expanded it a little bit with more specific bluster information but kept most of what you had.

[jashapiro]

Looks good! Just a couple wording/format changes. Which will require rebuilding docs, unfortunately.