Skip to content

Commit

Permalink
Fix issue 3793 (#3843)
Browse files Browse the repository at this point in the history
* Demonstrate issue 3793.

* Give Data model-relative names.

Use Model.name_for() to relativize pm.Data names.

* Move test description into docstring

* mention changes to pm.Data naming

closes #3793

Co-authored-by: michaelosthege <thecakedev@hotmail.com>
  • Loading branch information
rpgoldman and michaelosthege authored Mar 20, 2020
1 parent 74b7788 commit 29821a5
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 7 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- Remove `sample_ppc` and `sample_ppc_w` that were deprecated in 3.6.
- Tuning results no longer leak into sequentially sampled `Metropolis` chains (see #3733 and #3796).
- Deprecated `sd` in version 3.7 has been replaced by `sigma` now raises `DepreciationWarning` on using `sd` in continuous, mixed and timeseries distributions. (see #3837 and #3688).
- In named models, `pm.Data` objects now get model-relative names (see [#3843](https://github.com/pymc-devs/pymc3/pull/3843))

## PyMC3 3.8 (November 29 2019)

Expand Down
19 changes: 12 additions & 7 deletions pymc3/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,6 @@ class Data:
https://docs.pymc.io/notebooks/data_container.html
"""
def __new__(self, name, value):
# `pm.model.pandas_to_array` takes care of parameter `value` and
# transforms it to something digestible for pymc3
shared_object = theano.shared(pm.model.pandas_to_array(value), name)

# To draw the node for this variable in the graphviz Digraph we need
# its shape.
shared_object.dshape = tuple(shared_object.shape.eval())

# Add data container to the named variables of the model.
try:
Expand All @@ -494,6 +487,18 @@ def __new__(self, name, value):
raise TypeError("No model on context stack, which is needed to "
"instantiate a data container. Add variable "
"inside a 'with model:' block.")

name = model.name_for(name)

# `pm.model.pandas_to_array` takes care of parameter `value` and
# transforms it to something digestible for pymc3
shared_object = theano.shared(pm.model.pandas_to_array(value), name)

# To draw the node for this variable in the graphviz Digraph we need
# its shape.
shared_object.dshape = tuple(shared_object.shape.eval())


model.add_random_variable(shared_object)

return shared_object
12 changes: 12 additions & 0 deletions pymc3/tests/test_data_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,15 @@ def test_model_to_graphviz_for_model_with_data_container(self):
assert text in g.source
text = 'obs [label="obs ~ Normal" style=filled]'
assert text in g.source


def test_data_naming():
"""
This is a test for issue #3793 -- `Data` objects in named models are
not given model-relative names.
"""
with pm.Model("named_model") as model:
x = pm.Data("x", [1.0, 2.0, 3.0])
y = pm.Normal("y")
assert y.name == "named_model_y"
assert x.name == "named_model_x"

0 comments on commit 29821a5

Please sign in to comment.