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

Create educational notebook based on Sargolini 2006 #26

Merged
merged 13 commits into from
Mar 25, 2024

Conversation

weiglszonja
Copy link
Contributor

@weiglszonja weiglszonja commented Jul 27, 2023

@weiglszonja weiglszonja marked this pull request as draft July 27, 2023 14:44
@bendichter
Copy link
Member

@weiglszonja what still needs to be done with this PR? Is it ready for review?

@weiglszonja
Copy link
Contributor Author

@bendichter Adding the cell layer information is missing currently, I checked the file that contains this, should not be hard to match it with the existing data. I can do it this week.

@weiglszonja weiglszonja marked this pull request as ready for review September 16, 2023 14:40
@weiglszonja
Copy link
Contributor Author

@bendichter ready for review

@bendichter
Copy link
Member

What is the purpose of

def pearson_cor_2arr(arr1, arr2):
    """ Calculating Pearson correlation for two arrays with the same shape and no NaN."""
    if not arr1.shape == arr2.shape:
        raise Exception("Both arrays should have the same shape")
    n = arr1.shape[0] * arr1.shape[1]
 
    numerator = n * np.sum(arr1 * arr2) - np.sum(arr1) * np.sum(arr2)
    denominator = (n * np.sum(arr1**2) - np.sum(arr1)**2)**0.5 * (n * np.sum(arr2**2) - np.sum(arr2)**2)**0.5
    return numerator / denominator

I think this should be the same as scipy.stats.pearsonr(arr1[:], arr2[:])[0]

@weiglszonja
Copy link
Contributor Author

weiglszonja commented Sep 18, 2023

What is the purpose of

def pearson_cor_2arr(arr1, arr2):
    """ Calculating Pearson correlation for two arrays with the same shape and no NaN."""
    if not arr1.shape == arr2.shape:
        raise Exception("Both arrays should have the same shape")
    n = arr1.shape[0] * arr1.shape[1]
 
    numerator = n * np.sum(arr1 * arr2) - np.sum(arr1) * np.sum(arr2)
    denominator = (n * np.sum(arr1**2) - np.sum(arr1)**2)**0.5 * (n * np.sum(arr2**2) - np.sum(arr2)**2)**0.5
    return numerator / denominator

I think this should be the same as scipy.stats.pearsonr(arr1[:], arr2[:])[0]

You're right, the arrays just have to be flattened, I don't remember why we didn't use pearsonr (let me know @djarecka if you do). Changed it in the notebook

@yarikoptic
Copy link
Member

looks good to me. @bendichter do you give your blessing ? then please click big green button!

@djarecka
Copy link
Member

What is the purpose of

def pearson_cor_2arr(arr1, arr2):
    """ Calculating Pearson correlation for two arrays with the same shape and no NaN."""
    if not arr1.shape == arr2.shape:
        raise Exception("Both arrays should have the same shape")
    n = arr1.shape[0] * arr1.shape[1]
 
    numerator = n * np.sum(arr1 * arr2) - np.sum(arr1) * np.sum(arr2)
    denominator = (n * np.sum(arr1**2) - np.sum(arr1)**2)**0.5 * (n * np.sum(arr2**2) - np.sum(arr2)**2)**0.5
    return numerator / denominator

I think this should be the same as scipy.stats.pearsonr(arr1[:], arr2[:])[0]

You're right, the arrays just have to be flattened, I don't remember why we didn't use pearsonr (let me know @djarecka if you do). Changed it in the notebook

@weiglszonja - sorry, missed the question. Don't remember, why I created the function on my own, perhaps had some issues with the scipy function or wanted to compare. we can check if this gives the same results now.

@weiglszonja
Copy link
Contributor Author

@djarecka no worries, I checked already.

@CodyCBakerPhD
Copy link
Contributor

@weiglszonja @bendichter Any updated on this? Is this ready?

@weiglszonja
Copy link
Contributor Author

@CodyCBakerPhD This is good to go from my side, but I don't have permission to merge.

@satra satra merged commit e87f203 into dandi:master Mar 25, 2024
1 check passed
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.

6 participants