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

Cell-annotation fix for AUCell #6

Closed
wants to merge 1 commit into from

Conversation

alanocallaghan
Copy link
Collaborator

So a) sorry for the unintended pushes to master; I thought I'd added my fork as a remote but I'd actually just added osca-source twice.

Should we assume that AUCell aren't going to accept this PR after ~6 months? aertslab/AUCell#28

Obviously a breaking change for no reason is not good; maybe it'd be better to try removing it as a dependency and finding an alternative?

@hpages
Copy link
Contributor

hpages commented Oct 7, 2022

Should we assume that AUCell aren't going to accept this PR after ~6 months? aertslab/AUCell#28

Just to be clear, this is a link to an issue that discusses the unnecessary breaking changes that the AUCell developers have introduced to their package in BioC 3.16. AFAIK, no PR was submitted to their package to address that.

But I agree that after 6 months we can assume that they're not interested in fixing the problem they've introduced 😞

maybe it'd be better to try removing it as a dependency and finding an alternative?

Absolutely! Especially if the stability of the OSCA book is a concern. Also, as an educational tool, the book should only pick state of the art tools that adhere to good software engineering practices. I don't think a function like AUCell_buildRankings() falls in that category.

Thanks for dealing with this.

H.

@alanocallaghan
Copy link
Collaborator Author

Ah true, I was reading the proposed changes as a diff somehow.

The alternative then would be to make a PR, but I don't see that being hugely useful; in my experience a PR is likely to sit for months as well.

I'll have a look at what buildrankings does at the weekend

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.

2 participants