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

Hotspots and help(SpaceMarkersFunction) #48

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

orian116
Copy link
Collaborator

@orian116 orian116 commented Nov 17, 2024

  1. Hotspots:
    a) Change find_pattern_hotspots to camel case findPatternHotspots.
    b) changed kernelthreshold default from 3 to 4.
    c)Exported findPatternHotspots function
    d) Added examples and tests in testthat

  2. Updated SpaceMarkers functions description for more readable titles in R help menu. Did not make changes to the actual functions

  3. Removed files associated with getSpatialParametersExternal and replaced with getSpatialParamsExternal

orian116 and others added 5 commits November 4, 2024 18:10
… sigma (lowres as default). Incorperated changes from main branch intp SpaceMarkers_vignette including new plotting functions, getSpatialParametersExternal and getPairwiseInteractingGenes
…resolution scaling as specified in getSpatialParamsExternal.
…changed kernelthreshold default from 3 to 4. c) Exported findPatternHotspots function d) Added examples and tests in testthat 2. Updated functions' documentation for cleaner and more readable text in R's help menu
@@ -1,13 +1,75 @@
## author: Atul Deshpande
## email: [email protected]

find_pattern_hotspots <- function(
#===================
#' @title Calculate influence of spatially varying features
Copy link
Member

Choose a reason for hiding this comment

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

Identify hotspots of spatial pattern influence

@@ -1,13 +1,75 @@
## author: Atul Deshpande
## email: [email protected]

find_pattern_hotspots <- function(
Copy link
Contributor

Choose a reason for hiding this comment

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

Great, it's comforting to see we're getting back to harmonized fun names

spPatterns, params = NULL, patternName = "Pattern_1",
outlier = "positive",
nullSamples = 100, includeSelf = TRUE,...){
if (is.null(params)){
sigmaPair <- 10
kernelthreshold <- 3
kernelthreshold <- 4
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why not 5?

if (!all(patternPairs %in% patternList))
stop("Following are not pattern names: ",
paste0(setdiff(patternPairs, patternList)," "))
stop("Following are not pattern names: ",mess )
Copy link
Contributor

@dimalvovs dimalvovs Nov 18, 2024

Choose a reason for hiding this comment

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

stop mess :) (just noticing, no call to action)

@@ -139,10 +135,12 @@ getSpatialParameters <- function(spatialPatterns,...){
#' @param spatialDir A string path specifying the location of the spatial folder
#' containing the .json file of the spot characteristics
#' @param pattern A string specifying the name of the .json file
#' @param spotDiameter A numeric value specifying your desired sigma
#' @param sigma A numeric value specifying your desired sigma
Copy link
Contributor

@dimalvovs dimalvovs Nov 18, 2024

Choose a reason for hiding this comment

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

I'd rather note what sigma is and how it affects the result instead of kind of obvious "A numeric value specifying your desired sigma"

@@ -19,11 +19,11 @@ test_that("getInteracting genes return empty interacting_genes object when no in
"Pattern_2" = runif(cell_num , min=0, max=1))

optParams <- NULL
output <- getInteractingGenes(data=data, spPatterns, reconstruction=NULL,
suppressWarnings(output <- getInteractingGenes(data=data, spPatterns, reconstruction=NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is warning that we suppress here? Do we risk suppressing something unwanted?

tissue_hires_scalef = 2,
tissue_lowres_scalef = 3,
tissue_hires_scalef = 0.75,
tissue_lowres_scalef = 0.25,
spot_diameter_fullres = 4
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticing - if we changed the spotDiameter to sigma elsewhere, do we need to do the same also here?

Copy link
Contributor

@dimalvovs dimalvovs left a comment

Choose a reason for hiding this comment

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

Looks OK to me, just 3 things maybe to consider

  • spotDiameter change to sigma: I can't really understand what sigma is, and how it affects the results;
  • supressWarnings: can it be that we'll suppress something important? What we're suppressing by the way?
  • spotDiameter changed to sigma in the fun, but not in the json_data, it it OK?

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.

3 participants