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

SyntaxWarnings which should be UserWarnings (?) #1014

Closed
gimbo opened this issue Jan 20, 2020 · 2 comments
Closed

SyntaxWarnings which should be UserWarnings (?) #1014

gimbo opened this issue Jan 20, 2020 · 2 comments

Comments

@gimbo
Copy link
Contributor

gimbo commented Jan 20, 2020

(I'm not sure if this is a bug per se, but it feels wrong to me — and I'm open to being wrong about that. I'm hesitant to report it as I'm a new user of arviz, but OTOH I'm an experienced enough python programmer that I thought perhaps I might have something to add. And if I don't, that's OK.)

In short, it seems to me that arviz is using SyntaxWarning in a bunch of places that have nothing to do with syntax, and that UserWarning would be a better choice in those cases.

How I came across this: I just tried to create an autocorrelation plot which, as it happened, consisted of 45 subplots. So I got this warning:

SyntaxWarning: rcParams['plot.max_subplots'] (40) is smaller than the number of variables to plot (45) in plot_autocorr, generating only 40 plots

I can work around it easily, of course, but I'm confused why it's a SyntaxWarning. The python docs say this is the "Base class for warnings about dubious syntax." As far as I can see there's no "dubious syntax" here. Nothing about the syntax of how I asked for this plot to be produced is at fault or needs changing; it's just that I happen to have a Dataset which unfolds to 45 plots.

Shouldn't this be a UserWarning?

It looks like there are 54 instances of SyntaxWarning in the codebase (some in tests). Is it really intentional to use SyntaxWarning in these cases? Or was it just an accidental historical choice by someone which somehow stuck? Or maybe it's some convention I'm not aware of...? I don't see this in matplotlib, for example (but maybe I'm missing something...)

(BTW, ironically, I'm glad it was a SyntaxWarning because I happen to have suppressed UserWarnings in this notebook, and thus wouldn't have noticed that I was only getting 40 plots instead of the expected 45. But I think that's incidental and it's still a good idea to use the appropriate warning type in general... :-) )

@ahartikainen
Copy link
Contributor

I think you are right. Do you want to create a PR?

@gimbo
Copy link
Contributor Author

gimbo commented Jan 21, 2020

OK, done: PR 1016.

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

3 participants