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

"Error in plot.window(...) : need finite 'ylim' values" plot,Chromatograms #249

Closed
jorainer opened this issue Sep 5, 2017 · 8 comments
Closed

Comments

@jorainer
Copy link
Collaborator

jorainer commented Sep 5, 2017

Error in plot.window(...) : need finite 'ylim' values is thrown in plot,Chromatograms if none of the Chromatogram objects contain valid values (i.e. only NA intensities for the provided rtime).

I would like to catch this error and draw an empty plot with ylim = c(1, 0). That OK with you @lgatto ?

Note: this applies also to plot,Chromatogram.

@lgatto
Copy link
Owner

lgatto commented Sep 5, 2017

What about an isEmpty,Chromatogram[s] to test the data in the plot method, and return either a message or a warning before producing an empty plot?

@jorainer
Copy link
Collaborator Author

jorainer commented Sep 5, 2017

Good point. I'll implement isEmpty,Chromatogram[s]. So, the approach would be to check if the chromatogram is empty and if it is show a warning and still produce the plot (but with ylim = c(0, 1).
IMHO a plot call should, in case there is e.g. no data, show a plot (and a warning) but not stop with an error.

@lgatto
Copy link
Owner

lgatto commented Sep 5, 2017

Yes, I agree that an empty plot after a warning is more friendly than an error. The plot could even have empty chromatogram text on it, to make it obvious.

jorainer added a commit that referenced this issue Sep 5, 2017
- plot,Chromatogram[s] creates and empty plot and returns a warning if the
  Chromatogram[s] object is empty (issue #249).
@jorainer
Copy link
Collaborator Author

jorainer commented Sep 5, 2017

Done - @lgatto are you also pushing to Bioc at some point?

@lgatto
Copy link
Owner

lgatto commented Sep 5, 2017

Will do as I have a bit of time - tonight or tomorrow morning.

@jorainer
Copy link
Collaborator Author

jorainer commented Sep 6, 2017

no hurry, was just asking.

@lgatto
Copy link
Owner

lgatto commented Sep 6, 2017

Well, I thought I would merge, but upstream vs origin is a f****ing mess. No idea how it got there, and how to sort it out. It will take some time, I'm afraid...

@lgatto
Copy link
Owner

lgatto commented Sep 7, 2017

On bioc now.

@lgatto lgatto closed this as completed Sep 7, 2017
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

No branches or pull requests

2 participants