-
-
Notifications
You must be signed in to change notification settings - Fork 417
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
negate variable names #574
Conversation
Can I like this feature more than once? This is a great idea |
Good idea. I'm assuming |
Thanks @canyon289 and @ahartikainen. Right names like "~a" are not valid. |
How can we handle this situation?
|
arviz/utils.py
Outdated
else: | ||
all_vars = list(data.data_vars) | ||
|
||
exclude_vars = [i[1:] for i in var_names if i.startswith("~")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exclude_vars = [i[1:] for i in var_names if i.startswith("~") and i not in all_vars]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will give the wrong result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ups, sorry I misread that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, with this change negation will work as expected except that if a variable name starts with ~
like ~mu
when passing ~mu
to var_names
it will understand that you want the variable ~mu
and not to negate a variable named mu
. Passing ~~mu
will be understood as negating the variable ~mu
. Is this OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that sounds logical.
The is the corner case where we have mu
and ~mu
... Maybe it should throw a warning if there are both parameters: par
and ~par
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But... how do you negate mu
in that situation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're saying if a model contains both mu
and ~mu
how would you negative one but not the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you negate mu. I'm not saying we need to handle it, but warn users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I think that's reasonable. Scan the var_names in InferenceData and see if any are leading with a tilda. If so provide warning negation behavior might be weird.
If people submit tickets about warning and want a "better" we can then decide to solve further?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, I missed this conversation, I will add the warning.
LGTM |
This allow to pass a variable to
var_names
and get all the variables except the one that was passed.Thus if
data
is a model with variables names ['mu', 'theta', 'tau']returns
One caveat is that variables names are then no allowed to starts with
~
.