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

Kmeans notebook cleanup #946

Merged
merged 16 commits into from
Mar 23, 2023
Merged

Kmeans notebook cleanup #946

merged 16 commits into from
Mar 23, 2023

Conversation

camisowers
Copy link
Contributor

@camisowers camisowers commented Mar 14, 2023

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Closes #876.

How did you implement your changes

Added more clear descriptions of what is actually being shown by the generated heat maps. Also fixed a few pandas warning messages that were clogging up the notebook outputs.

The visualizations were all updated as well to reflect the example dataset with a cropped fov1.

Remaining issues
If anyone has specific additions they want added into the kmeans notebook, let me know!

  • Possibly add more visualizations specifically for if someone used neighbor frequencies as their input.
  • The kmeans notebook always regenerates the neighborhood matrix. It might be helpful to be able to simply read in a previously generated matrix, like how we have in the mixing score notebook.

@camisowers camisowers added the enhancement New feature or request label Mar 14, 2023
@camisowers camisowers self-assigned this Mar 14, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@camisowers camisowers requested review from cliu72 and ngreenwald March 14, 2023 20:49
@cliu72
Copy link
Contributor

cliu72 commented Mar 16, 2023

I think it looks great! Just some minor suggestions for wording, but open to other suggestions. I think being able to read in an existing neighborhood matrix is a good idea.

@camisowers camisowers requested review from srivarra and removed request for ngreenwald March 21, 2023 19:12
Copy link
Contributor

@srivarra srivarra left a comment

Choose a reason for hiding this comment

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

Everything looks good! Just a small question, and I'm looking into why our tests are failing currently.

src/ark/analysis/neighborhood_analysis.py Outdated Show resolved Hide resolved
@camisowers camisowers requested a review from ngreenwald March 21, 2023 21:42
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Looks great, almost ready

@camisowers camisowers requested a review from ngreenwald March 23, 2023 17:36
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Looks good!

@ngreenwald ngreenwald added this pull request to the merge queue Mar 23, 2023
Merged via the queue into main with commit 8e53667 Mar 23, 2023
@ngreenwald ngreenwald deleted the neighborhood_nb_cleanup branch March 23, 2023 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Neighborhood notebook updates
4 participants