Qqplot bug

[roskamsh]

Branch to fix bug in TargeneCore.jl in which qqplot() function threw in error when only a single p-value was present in the results. This branch checks if the number of non-NA p-values are less than or equal to 1 and does not return a QQplot.png if this is the case. Tests also added.

[joshua-slaughter]

Looks good to me. Nice catch!

[olivierlabayle]

Thanks breeshey, I think a slight adjustment is necessary as explained in my comments. The test will probably have to be updated as well since a file will be created.

[roskamsh]

Thanks breeshey, I think a slight adjustment is necessary as explained in my comments. The test will probably have to be updated as well since a file will be created.

What do you mean a file will be created? In the current implementation, the number of p-values = 1, and so a QQplot is not generated. This is still consistent with the test I've implemented?

[olivierlabayle]

Thanks breeshey, I think a slight adjustment is necessary as explained in my comments. The test will probably have to be updated as well since a file will be created.

What do you mean a file will be created? In the current implementation, the number of p-values = 1, and so a QQplot is not generated. This is still consistent with the test I've implemented?

If you use continue instead of return, I believe the plot will be created even if it will be empty.

[roskamsh]

Thanks breeshey, I think a slight adjustment is necessary as explained in my comments. The test will probably have to be updated as well since a file will be created.

What do you mean a file will be created? In the current implementation, the number of p-values = 1, and so a QQplot is not generated. This is still consistent with the test I've implemented?

If you use continue instead of return, I believe the plot will be created even if it will be empty.

Why would we want to create an empty file?

We can make the targene-pipeline adaptable to this context by adding it as an optional output. See: TARGENE/targene-pipeline@6835f0e

I see your comment about one of the estimators having sufficient values, but maybe we can not create a QQ plot if all estimators have less than or equal to 1 non-NA p-value?

[olivierlabayle]

Thanks breeshey, I think a slight adjustment is necessary as explained in my comments. The test will probably have to be updated as well since a file will be created.

What do you mean a file will be created? In the current implementation, the number of p-values = 1, and so a QQplot is not generated. This is still consistent with the test I've implemented?

If you use continue instead of return, I believe the plot will be created even if it will be empty.

Why would we want to create an empty file?

We can make the targene-pipeline adaptable to this context by adding it as an optional output. See: TARGENE/targene-pipeline@6835f0e

I see your comment about one of the estimators having sufficient values, but maybe we can not create a QQ plot if all estimators have less than or equal to 1 non-NA p-value?

The goal is not to create an empty file but to leave the possibility for other estimators to have sufficiently many pvalues to be plotted. Regarding the case where no estimator has sufficiently many pvalues and the plot would be empty, this does not seem particulalry bad to me (from a user perspective, as compared to no plot file at all). If you have a strong opinion on this I'm happy to discuss. This is a bit more work and you will also have to update the documentation.

[roskamsh]

Thanks breeshey, I think a slight adjustment is necessary as explained in my comments. The test will probably have to be updated as well since a file will be created.

What do you mean a file will be created? In the current implementation, the number of p-values = 1, and so a QQplot is not generated. This is still consistent with the test I've implemented?

If you use continue instead of return, I believe the plot will be created even if it will be empty.

Why would we want to create an empty file?
We can make the targene-pipeline adaptable to this context by adding it as an optional output. See: TARGENE/targene-pipeline@6835f0e
I see your comment about one of the estimators having sufficient values, but maybe we can not create a QQ plot if all estimators have less than or equal to 1 non-NA p-value?

The goal is not to create an empty file but to leave the possibility for other estimators to have sufficiently many pvalues to be plotted. Regarding the case where no estimator has sufficiently many pvalues and the plot would be empty, this does not seem particulalry bad to me (from a user perspective, as compared to no plot file at all). If you have a strong opinion on this I'm happy to discuss. This is a bit more work and you will also have to update the documentation.

I see, I see. Okay - I guess the likeliness that this will happen is probably quite low anyways, as TarGene is most likely to be run across more than a single variant and single trait. I can go ahead with your suggestion and will update here!

[olivierlabayle]

Perfect, many thanks!