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

enables highly_variable_genes_seurat_v3 to accept pseudocounts #1679

Merged
merged 10 commits into from
Feb 24, 2021
Merged

Conversation

giovp
Copy link
Member

@giovp giovp commented Feb 23, 2021

fixes #1642 .

I'll reply to the comments here @adamgayoso

shouldn't both not be Optional?

I removed optional type from both span and check_values

On another note, in this current state check_nonnegative_integers is an unused import. Do you guys check for this with flake8?

we don't have pre-commit in place, we are discussing it here #1563 . I do check flake8 but clearly didn't do it this time.

I think a bug has been introduced @ivirshup @giovp. Namely, the call to check_nonnegative_integers(X) was removed (should be here on line 61) if returns False, raise the warning

this should be fixed now

Sorry again for very sloppy handling of this, should be ready to review

@giovp giovp requested a review from ivirshup February 23, 2021 11:02
Copy link
Member

@adamgayoso adamgayoso left a comment

Choose a reason for hiding this comment

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

LGTM

@adamgayoso
Copy link
Member

we don't have pre-commit in place, we are discussing it here #1563 . I do check flake8 but clearly didn't do it this time.

I thought at one point you guys were checking flake8 with CI.

@ivirshup
Copy link
Member

I thought at one point you guys were checking flake8 with CI.

We were, sorta. The CI tool we were using had pretty stochastic reporting (which isn't really what we want in a CI tool). Hopefully it'll be back in a more reliable form soon: #1563 (comment)

Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

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

Some minor style comments (up to you on making the changes), but looks good overall.

scanpy/preprocessing/_highly_variable_genes.py Outdated Show resolved Hide resolved
scanpy/preprocessing/_highly_variable_genes.py Outdated Show resolved Hide resolved
giovp and others added 2 commits February 24, 2021 13:04
Co-authored-by: Isaac Virshup <ivirshup@gmail.com>
Co-authored-by: Isaac Virshup <ivirshup@gmail.com>
@giovp giovp merged commit f7279f6 into master Feb 24, 2021
@giovp giovp deleted the high-var branch February 24, 2021 12:57
@ivirshup
Copy link
Member

ivirshup commented Apr 7, 2021

@meeseeksdev backport to 1.7.x

@lumberbot-app
Copy link

lumberbot-app bot commented Apr 7, 2021

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout 1.7.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 f7279f6342f1e4a340bae2a8d345c1c43b2097bb
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #1679: enables highly_variable_genes_seurat_v3 to accept pseudocounts'
  1. Push to a named branch :
git push YOURFORK 1.7.x:auto-backport-of-pr-1679-on-1.7.x
  1. Create a PR against branch 1.7.x, I would have named this PR:

"Backport PR #1679 on branch 1.7.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

ivirshup added a commit that referenced this pull request Apr 7, 2021
Co-authored-by: Isaac Virshup <ivirshup@gmail.com>
ivirshup added a commit that referenced this pull request Apr 7, 2021
* Set release date
* Remove unused rubrics from release notes
* enables highly_variable_genes_seurat_v3 to accept pseudocounts (#1679) 
* Add hvg change to release notes

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>
Co-authored-by: giovp <giov.pll@gmail.com>
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.

4 participants