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

Narrow peak debug #127

Merged
merged 14 commits into from
Nov 5, 2019
Merged

Narrow peak debug #127

merged 14 commits into from
Nov 5, 2019

Conversation

LeilyR
Copy link
Contributor

@LeilyR LeilyR commented Oct 31, 2019

fixed a bug in narrowPeaks which brought to us by #120

Leily Rabbani and others added 6 commits October 29, 2019 16:14
* added info in the doc of get_scores

* integrate the cases when there is no item in the region

* updated output of make_tracks_file
* added info in the doc of get_scores

* integrate the cases when there is no item in the region

* updated output of make_tracks_file

* bedgraph inherit the init from GenomeTracks to have self.log
Copy link
Collaborator

@lldelisle lldelisle left a comment

Choose a reason for hiding this comment

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

Hi,
I think we should agree on the return of get_score.
Initially, when the chromosome was not on the file it was raising an error and when it was part of the file but there was no item in the region it was returning [nan], (start_region, end_region).
I guess this was to be able to make bedgraph to plot correctly.
So, if we keep this behaviour, when the chromosome is not part of the file, it should return [nan], (start_region, end_region).

The class which use this function are:
bedgraph ok
bedgraphmatrix ok
epilogos ok
narrowPeaks.
In the case of narrowPeaks, for idx, peak in enumerate(score_list): will go once except if we test before that the score_list is not [nan]...

@@ -174,7 +175,7 @@ def get_scores(self, chrom_region, start_region, end_region, return_nans=True):
"chromosome name inside the bedgraph "
"file. This will generate an empty "
"track!!\n")
return
return score_list, pos_list
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should return the same thing line 161.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, I need to change that error message to warning too.

@LeilyR
Copy link
Contributor Author

LeilyR commented Oct 31, 2019

Hi,
I think we should agree on the return of get_score.
Initially, when the chromosome was not on the file it was raising an error and when it was part of the file but there was no item in the region it was returning [nan], (start_region, end_region).
I guess this was to be able to make bedgraph to plot correctly.
So, if we keep this behaviour, when the chromosome is not part of the file, it should return [nan], (start_region, end_region).

The class which use this function are:
bedgraph ok
bedgraphmatrix ok
epilogos ok
narrowPeaks.
In the case of narrowPeaks, for idx, peak in enumerate(score_list): will go once except if we test before that the score_list is not [nan]...

I will check for them, thanks!

@lldelisle
Copy link
Collaborator

Hi,
Sorry, it is a misunderstanding. As I saw that you used return and in plot you used:

        if pos_list == []:
            return

I thought you would prefer that the default is an empty list.
No problem to put it back to nan. Do you want to do it or I make PR?

@LeilyR
Copy link
Contributor Author

LeilyR commented Nov 4, 2019

Either is fine by me, I would leave it be as long as it works fine. Thanks for the clarification!

@lldelisle
Copy link
Collaborator

I prefer to let it empty this way we do not need to import numpy to check if it is equal to np.nan...

@LeilyR LeilyR merged commit 56f8ead into develop Nov 5, 2019
@LeilyR LeilyR deleted the narrowPeak_debug branch November 5, 2019 09:31
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