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

Change raster warning only if not explicitly supplied #7842

Closed

Conversation

samuel-marsh
Copy link
Collaborator

Hi Seurat Team,

This is small change to clean up console output. Currently plots based on SingleExIPlot or SingleDimPlot issue a raster warning whenever the raster parameter is not FALSE and the number of points is over 100K. However, because of this it also continuously prints the message even when user explicitly sets the parameter to be TRUE. This can clutter output especially if return lots of plots in loop for something the user has explicitly set. This was mentioned previously in #7745. While I know that you can suppress such warnings many users may not.

This PR simply changes the !isFALSE(raster) to is.null(raster) since NULL is the default setting. Therefore it will only print warning when setting is not explicitly provided by the user.

This may have been specific design decision to warn every time plot is rasterized but seems unnecessary if user is supplying parameter. If it is specific design decision then feel free to ignore and close this PR.

Thanks as always,
Sam

@saketkc
Copy link
Collaborator

saketkc commented Sep 27, 2023

Thank you @samuel-marsh! I have implemented the change in 643a57d. Since we were about to release 4.4.0, we decided to do this internally rather then merging your PR. Thank you for your PR!

@saketkc saketkc closed this Sep 27, 2023
@samuel-marsh
Copy link
Collaborator Author

Hi @saketkc, Sounds good, thanks!!

@samuel-marsh
Copy link
Collaborator Author

samuel-marsh commented Sep 28, 2023

Hi @saketkc,

REOPENING to ensure this is seen but feel free to close after that

I just wanted to let you know for when you propagate changes to the V5 branch (or potentially hotfix the 4.4.0 branch) there is one error in the raster changes in the 4.4.0 branch. It was actually plot I hadn't caught (SingleCorPlot).

The exclamation point before the is.null needs to be removed.

if ((nrow(x = data) > 1e5) & !is.null(x = raster)){

Right now it will warn when explicitly TRUE or FALSE.

Best,
Sam

@samuel-marsh samuel-marsh reopened this Sep 28, 2023
@saketkc
Copy link
Collaborator

saketkc commented Sep 28, 2023

Thanks Sam! I missed it too. We might release a hotfix and propogate to V5 (the latest V5 is synced up with V4.4.0)

@samuel-marsh
Copy link
Collaborator Author

Sounds good! I'll close this. Thanks again!

@samuel-marsh samuel-marsh mentioned this pull request Oct 17, 2023
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.

2 participants