-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Use arviz 1.0 #8019
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
base: main
Are you sure you want to change the base?
Use arviz 1.0 #8019
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Tests are passing locally, except for |
OriolAbril
left a comment
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
Description
This makes the changes to use arviz 1.0. Not all tests are passing yet.
Checklist
Type of change