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

Fix: Extraction of matrix_cell_count is wrong (#3097) #3101

Conversation

dsotirho-ucsc
Copy link
Contributor

@dsotirho-ucsc dsotirho-ucsc commented Jun 2, 2021

Author

  • PR title references issue
  • Title of main commit references issue
  • PR is linked to Zenhub issue

Author (reindex)

  • Added r tag to commit title or this PR does not require reindexing
  • Added reindex label to PR or this PR does not require reindexing

Author (freebies & chains)

  • Freebies are blocked on this PR or there are no freebies in this PR
  • Freebies are referenced in commit titles or there are no freebies in this PR
  • This PR is blocked by previous PR in the chain or this PR is not chained to another PR
  • Added chain label to the blocking PR or this PR is not chained to another PR

Author (upgrading)

  • Documented upgrading of deployments in UPGRADING.rst or this PR does not require upgrading
  • Added u tag to commit title or this PR does not require upgrading
  • Added upgrade label to PR or this PR does not require upgrading

Author (requirements, before every review)

  • Ran make requirements_update or this PR leaves requirements*.txt, common.mk and Makefile untouched
  • Added R tag to commit title or this PR leaves requirements*.txt untouched
  • Added reqs label to PR or this PR leaves requirements*.txt untouched

Author (before every review)

  • make integration_test passes in personal deployment or this PR does not touch functionality that could break the IT
  • Rebased branch on develop, squashed old fixups

Primary reviewer (after approval)

  • Commented in issue about demo expectations or labelled issue as no demo
  • Decided if PR can be labeled no sandbox
  • PR title is appropriate as title of merge commit
  • Moved ticket to Approved column
  • Assigned PR to an operator

Operator (before pushing merge the commit)

  • Checked reindex label and r commit title tag
  • Checked that demo expectations are clear or issue is labeled as no demo
  • Rebased and squashed branch
  • Sanity-checked history
  • Pushed PR branch to Github
  • Branch pushed to Gitlab or PR is labeled no sandbox
  • Build passes in sandbox and added sandbox label or PR is labeled no sandbox
  • Started reindex in sandbox or this PR does not require reindexing sandbox
  • Checked for failures in sandbox or this PR does not require reindexing sandbox
  • Added PR reference to merge commit title
  • Collected commit title tags in merge commit title
  • Moved linked issue to Merged column
  • Pushed merge commit to Github

Operator (after pushing the merge commit)

  • Moved freebies to Merged column or there are no freebies in this PR
  • Shortened the PR chain or this PR is not the base of another PR
  • Verified that N reviews labelling is accurate
  • Pushed merge commit to Gitlab or this changes can be pushed later, together with another PR
  • Deleted PR branch from Github and Gitlab

Operator (reindex)

  • Started reindex in dev or this PR does not require reindexing or does not target dev
  • Checked for failures in dev or this PR does not require reindexing or does not target dev
  • Started reindex in prod or this PR does not require reindexing or does not target prod
  • Checked for failures in prod or this PR does not require reindexing or does not target prod

Operator

  • Unassigned PR

@github-actions github-actions bot added the orange [process] Done by the Azul team label Jun 2, 2021
@dsotirho-ucsc dsotirho-ucsc force-pushed the issues/danielsotirhos/3097-fix-extract-matrix-cell-count-is-wrong branch from 7d0fea1 to 0cbfd6b Compare June 3, 2021 15:31
@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #3101 (e408e18) into develop (17c5354) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3101   +/-   ##
========================================
  Coverage    82.50%   82.50%           
========================================
  Files          115      115           
  Lines        12279    12279           
========================================
  Hits         10131    10131           
  Misses        2148     2148           
Impacted Files Coverage Δ
src/azul/plugins/metadata/hca/aggregate.py 97.72% <ø> (ø)
src/azul/plugins/metadata/hca/transform.py 99.27% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17c5354...e408e18. Read the comment docs.

@coveralls
Copy link

coveralls commented Jun 3, 2021

Coverage Status

Coverage remained the same at 83.078% when pulling e408e18 on issues/danielsotirhos/3097-fix-extract-matrix-cell-count-is-wrong into 17c5354 on develop.

@dsotirho-ucsc dsotirho-ucsc force-pushed the issues/danielsotirhos/3097-fix-extract-matrix-cell-count-is-wrong branch from 0cbfd6b to 1238ea0 Compare June 3, 2021 16:52
@dsotirho-ucsc dsotirho-ucsc added the reindex:dev [process] PR requires reindexing dev label Jun 3, 2021
Copy link
Member

@achave11-ucsc achave11-ucsc left a comment

Choose a reason for hiding this comment

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

LGTM. Approved!

@achave11-ucsc achave11-ucsc removed their assignment Jun 3, 2021
'matrix_cell_count': getattr(file, 'matrix_cell_count', None),
**(
{
'matrix_cell_count': file.matrix_cell_count,
Copy link
Member

Choose a reason for hiding this comment

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

Comma necessary?

@dsotirho-ucsc dsotirho-ucsc force-pushed the issues/danielsotirhos/3097-fix-extract-matrix-cell-count-is-wrong branch from 1238ea0 to ef70332 Compare June 3, 2021 18:39
@hannes-ucsc hannes-ucsc added the 0 reviews [process] Lead didn't request any changes label Jun 8, 2021
@hannes-ucsc hannes-ucsc removed their assignment Jun 8, 2021
@hannes-ucsc hannes-ucsc added the no demo [process] Not to be demonstrated at the end of the sprint label Jun 8, 2021
@nadove-ucsc nadove-ucsc force-pushed the issues/danielsotirhos/3097-fix-extract-matrix-cell-count-is-wrong branch from ef70332 to e408e18 Compare June 8, 2021 07:33
@nadove-ucsc nadove-ucsc added the sandbox [process] Resolution is being verified in sandbox deployment label Jun 8, 2021
@nadove-ucsc
Copy link
Contributor

No failures after reindexing sandbox.

@nadove-ucsc nadove-ucsc merged commit ce725fb into develop Jun 8, 2021
@nadove-ucsc nadove-ucsc deleted the issues/danielsotirhos/3097-fix-extract-matrix-cell-count-is-wrong branch June 8, 2021 23:05
@nadove-ucsc
Copy link
Contributor

Failures after reindexing dev:

40733888-3b9f-500e-84fd-1a18a64aadbe
94565940-7b2f-5fd7-beb1-dc309687fe09
9d0f5cd1-0ef6-5eed-8dd7-f15d36a34185
ae338c4e-650f-545b-8631-3c8d755aa2f9
c12a6ca2-3234-5182-ad0b-f8c2fb881ab5

Which match the expected failures from #2870 and #2873.

@nadove-ucsc nadove-ucsc removed their assignment Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 reviews [process] Lead didn't request any changes no demo [process] Not to be demonstrated at the end of the sprint orange [process] Done by the Azul team reindex:dev [process] PR requires reindexing dev sandbox [process] Resolution is being verified in sandbox deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants