Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
aloctavodia
force-pushed
the
arviz
branch
2 times, most recently
from
d63b5c0 to
312ff6b
Compare
|
Tests are passing locally, except for |
OriolAbril
left a comment
OriolAbril
left a comment
There was a problem hiding this comment.
I haven't finished to go over all the files, will try to finish soon
| dependencies: | ||
| # Base dependencies | ||
| - arviz>=0.13.0 | ||
| - arviz>=1.0.0rc0 |
There was a problem hiding this comment.
if we need to run the test suite before we make the stable 1.0 release we can add arviz-plots here in the conda section and then this arviz indicator with the release candidate on the pip section. If we use the rc part in the version pin for pip it should automatically take pre-releases into account.
| "Covariance": ":mod:`Covariance <pymc.gp.cov>`", | ||
| "Mean": ":mod:`Mean <pymc.gp.mean>`", | ||
| "InferenceData": ":class:`~arviz.InferenceData`", | ||
| "DataTree": ":class:`~xarray.DataTree`", |
There was a problem hiding this comment.
This is also something I am unsure about for arviz docs but we might want to keep the alias around for InferenceData and maybe point to the schema page or a page about the schema implementation in python arviz. All "InferenceData" objects are compatible with DataTree but not the other way around so I think there is still value in using "InferenceData" in docs as in addition to saying the python type is xarray.DataTree it also says it will follow the schema (i.e. if a posterior group is present it has a specific meaning, log_likelihood group has that other meaning, fit_info group could also be there but has no convention agreed-on meaning)
| # FIXME: Would be better to drop coordinates altogether, but arviz defaults to `np.arange(var_length)` | ||
| return dict_to_dataset(vars_dict, *args, dims=dims, coords=safe_coords, **kwargs) | ||
| return dict_to_dataset( | ||
| vars_dict, *args, dims=dims, coords=safe_coords, sample_dims=sample_dims, **kwargs | ||
| ) |
There was a problem hiding this comment.
I think this fixme is related to arviz-devs/arviz-base#79 in case someone wants to lend a hand.
| _zarr_available = False | ||
|
|
||
|
|
||
| WARMUP_TAG = "warmup_" |
There was a problem hiding this comment.
should we keep this variable somewhere in arviz-base even if we delete the custom InferenceData class?
| idata.extend(idata_pp) | ||
| idata.update(idata_pp) |
There was a problem hiding this comment.
I think update has the opposite behaviour extend used to have. The default in extend was a "left join". That is, whenever a group was present in both inputs the one in idata was kept, ignoring the one in idata_pp. update will take all groups in idata_pp and add them to idata, overwriting already present groups in idata. If this happens for observed_data which will generally be the conflicting one it doesn't matter in most cases but it might be worth to think about how we want things to behave while doing the update.
ricardoV94
added
trace-backend
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
3 participants
Description
This makes the changes to use arviz 1.0. Not all tests are passing yet.
Closes #7821
Checklist
Type of change