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

Plot fingerprint: return proper nans #1000

Conversation

dlaehnemann
Copy link
Contributor

Welcome to deepTools GitHub repository! Please check the following regarding
your pull request :

  • Does the PR contain new feature?
  • Does the PR contain bugfix?
  • Does the PR contain documentation changes?
  • Does the PR contain changes to the galaxy wrapper?

Rebased onto develop branch (compare PR #999 )

This problem came up when parsing the OutputQualityMetrics txt output file downstream when it contains the NA values in columns. To be exact, the conversion to float() in the multiqc module throws an error and leads to the respective plot being skipped. The conversion that fails is this line:
https://github.com/ewels/MultiQC/blob/9760f7561a56c1295a661471d7447abd085b8020/multiqc/modules/deeptools/plotFingerprint.py#L110

According to the documentation of float(), this should instead read nan to be parsed correctly. And this is what print(numpy.NAN) gives (also see numpy.NAN docs and what is used elsewhere in deeptools (a quick grep on the deeptools/ directory did not give any other results for "NA").

LeilyR and others added 8 commits April 16, 2020 16:04
changed iteritmes() in estimateEscaleFactor to its python3 compatible items()
* Fixes for 3.4.2

* disable azure on OSX

* Closes deeptools#945

* computeMatricOperation  dataRange (deeptools#951)

* Added dataRange to computeMatricOperation to return min,max,median and 10th and 90th percentile.

* fixed pep8

* more pep8 fix

* fixed test_sieve of azure tests

* one more fix for test_sieve

* imported pysam to test_readFiltering.py

* updated hash values for test_sieve

* fixed galaxy computeMatrixOperation.xml

* More fixes for galaxy wrapper

* fixed a little typo in bamCompare (deeptools#967)

* save the output matrix of the plotheatmap in a format to be compatible with running plotheatmap on it again (deeptools#968)

* Plot profile color map (deeptools#971)

* Now colors can be set by user for any heatmap in plotProfile. A bug in tick index for plotting heatmap is also fixed.

* added a small description

* pep8 fix

* added sortUsingSamples and clusterUsingSamples to the galaxy wrapper

* [WIP] added auto to plotheatmap deeptools#908 (deeptools#982)

* added auto to plotheatmap

* fixed lint, added warning message, updated the help for zmin, zmax

* galaxy test plotPCA

* lower down the delat for potPCA galaxy test

* fix deeptools#969 (deeptools#970)

* fix deeptools#969

* PEP8

* updated changes.txt

* fixed a little bug in parseCommon.py which caused flake8 failure.

* [WIP] added auto to plotheatmap deeptools#908 (deeptools#982)

* added auto to plotheatmap

* fixed lint, added warning message, updated the help for zmin, zmax

* galaxy test plotPCA

* lower down the delat for potPCA galaxy test

* updated version and changes

* removed alpha from plotPCA test

* removed compare="sim_size"


* fixed plotHeatmap.xml by removing size from the the params and adding anitizer for the 2 new params.

* upated change.txt

* updated the doc html

Co-authored-by: Devon Ryan <dpryan79@users.noreply.github.com>
@LeilyR
Copy link
Contributor

LeilyR commented Sep 17, 2020

thanks! Can you also merge our dev branch to your branch please?

@dlaehnemann
Copy link
Contributor Author

This should be up to date with your develop branch, as I rebased onto it. Also, it is Already up to date when I do:

git checkout develop
git pull origin
git checkout plot-fingerprint_return-proper-NANs
git merge develop

So I think this should be fine. I just fixed one of the test failures, as this was simply an expected testing output that needs to read nan instead of NA, now. But there is one remaining (and seemingly unrelated) error in the deeptools plotPCA tests, where some Eigenvalue seems to have changed:
https://dev.azure.com/dpryan79/dpryan79/_build/results?buildId=1791&view=logs&j=23cf4fef-bcb8-5d10-43e4-415f5b313c85&t=4321d142-d313-5ab7-1fea-ce80e4a1ce65&l=1046

No clue, what's happening there, and which value should be correct.

@dlaehnemann
Copy link
Contributor Author

Maybe I'll try another clean start from the develop branch.

@dlaehnemann dlaehnemann mentioned this pull request Sep 17, 2020
4 tasks
@dlaehnemann
Copy link
Contributor Author

Closing in favor of #1001

@dlaehnemann dlaehnemann deleted the plot-fingerprint_return-proper-NANs branch September 17, 2020 17:16
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