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

Updates to RunBanksy #162

Merged
merged 11 commits into from
Apr 5, 2024
Merged

Updates to RunBanksy #162

merged 11 commits into from
Apr 5, 2024

Conversation

jleechung
Copy link
Contributor

This PR implements:

  • a minor fix to feature handling.
  • computation of the scaled BANKSY matrix (using Seurat:::FastRowScale).
  • computation of the azimuthal Gabor filter.

@jleechung
Copy link
Contributor Author

Hi @mojaveazure ,

We are trying to push some updates to RunBanksy but the GitHub action checks are failing at the SeuratWrappers installation stage. Do you know how to resolve the errors? Let us know if there's anything we can do on our end.

Thank you!

@jleechung
Copy link
Contributor Author

Hi @dcollins15, we're trying to push some changes to BANKSY so users can directly install from this repo instead of our fork - but checks are failing at installation - do you know why this is the case? thank you!

@dcollins15 dcollins15 self-requested a review April 2, 2024 20:22
Copy link

@vipulsinghal02 vipulsinghal02 left a comment

Choose a reason for hiding this comment

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

Look good!

@dcollins15
Copy link
Contributor

Apologies for the long delay and thanks for the updates - these changes LGTM! 🥳

Updating the pre-merge check is slowly creeping up my ToDo list but you can ignore the failures for now 🙃

Before this is ready to be merged there are just a couple of small house-keeping things to take care of:

  1. Please rebase onto master
  2. Please update the package's DESCRIPTION to increment the version to 0.3.5 and also update the date

@jleechung
Copy link
Contributor Author

Hi @dcollins15, thanks for getting back - have updated DESCRIPTION and rebased. Please let me know if there's anything else to do. Thanks!

Copy link
Contributor

@dcollins15 dcollins15 left a comment

Choose a reason for hiding this comment

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

Wicked! Thanks @jleechung, I'll merge this in shortly 🙌

@jleechung
Copy link
Contributor Author

Thank you!

@dcollins15 dcollins15 merged commit 8d46d6c into satijalab:master Apr 5, 2024
1 check failed
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