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.

Sign up for GitHub

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

Jump to bottom

More doc fixes #3333

Merged
merged 3 commits into from
Sep 23, 2019
Merged

More doc fixes #3333

merged 3 commits into from
Sep 23, 2019

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Sep 22, 2019

While going through the docs, I noticed several issues:

  • in reshaping.rst, a code block gets hidden because of a missing newline
  • in api.rst, the section What parts of xarray are considered public API? is referenced (I think), but the link is broken.
  • the code samples in DataArray.plot are not recognized as such because of missing newlines
  • the documentation of Dataset.plot.scatter in the API reference is broken

This PR tries to fix all of those, but while the first three issues are easy to fix, I'm not sure whether my proposed fix for the one involving scatter is correct: I've added plot.dataset_plot.scatter to plot.__all__ (exposing xr.plot.scatter) and changed api.rst to reference plot.scatter instead of Dataset.plot.scatter. It seems harmless to me, but the information that scatter only exists for Dataset is less visible. Is that the way to fix this?

  • Passes black . && mypy . && flake8
  • Fully documented, including whats-new.rst for all changes and api.rst for new API

@dcherian
Copy link
Contributor

I've added plot.dataset_plot.scatter to plot.all (exposing xr.plot.scatter) and changed api.rst to reference plot.scatter instead of Dataset.plot.scatter. It seems harmless to me, but the information that scatter only exists for Dataset is less visible. Is that the way to fix this?

I don't know about this. We don't test for xr.plot.scatter. I don't know why adding Dataset.plot.scatter doesn't work. It would be good to track that down.

@dcherian
Copy link
Contributor

The rest looks good though. Thanks @keewis

@keewis
Copy link
Collaborator Author

keewis commented Sep 22, 2019 •
edited
Loading

Then either we have to start testing for it or edit the docs of Dataset.plot, because there that function is mentioned.

@keewis
Copy link
Collaborator Author

keewis commented Sep 22, 2019

I tried tracking it down, but it uses the same mechanism as DataArray.plot where a new instance of _PlotMethods is assigned on each getattr on the property. The documentation on DataArray.plot skips this issue entirely by pointing to xr.plot.

@dcherian
Copy link
Contributor

Yes we have similar issues with GroupBy, Resample and rolling objects. methods like mean etc. aren't shown in the docs. There must be some way to tell sphinx to look for these "injected" methods (I think that's what they're called).

How about you remove the scatter changes for now and we can merge the rest?

@keewis
Copy link
Collaborator Author

keewis commented Sep 23, 2019

done

@dcherian
Copy link
Contributor

Thanks!

@dcherian dcherian merged commit 6ec1da2 into pydata:master Sep 23, 2019
@keewis keewis deleted the doc-fixes branch September 23, 2019 10:46
dcherian added a commit that referenced this pull request Sep 24, 2019
@dcherian
Merge remote-tracking branch 'upstream/master' into scipy19-docs
aeb15b5 
* upstream/master: (43 commits)
  Add hypothesis support to related projects (#3335)
  More doc fixes (#3333)
  Improve the documentation of swap_dims (#3331)
  fix the doc names of the return value of swap_dims (#3329)
  Fix isel performance regression (#3319)
  Allow weakref (#3318)
  Clarify that "scatter" is a plotting method in what's new. (#3316)
  Fix whats-new date :/
  Revert to dev version
  Release v0.13.0
  auto_combine deprecation to 0.14 (#3314)
  Deprecation: groupby, resample default dim. (#3313)
  Raise error if cmap is list of colors (#3310)
  Refactor concat to use merge for non-concatenated variables (#3239)
  Honor `keep_attrs` in DataArray.quantile (#3305)
  Fix DataArray api doc (#3309)
  Accept int value in head, thin and tail (#3298)
  ignore h5py 2.10.0 warnings and fix invalid_netcdf warning test. (#3301)
  Update why-xarray.rst with clearer expression (#3307)
  Compat and encoding deprecation to 0.14 (#3294)
  ...
dcherian added a commit to dcherian/xarray that referenced this pull request Oct 3, 2019
@dcherian
Merge remote-tracking branch 'upstream/master' into map_blocks_2
aa4ea00 
* upstream/master:
  Remove setting of universal wheels (pydata#3367)
  Revisit # noqa annotations (pydata#3359)
  Fix codecov.io upload on Windows (pydata#3360)
  Add how do I ... section (pydata#3357)
  Add glossary to documentation (pydata#3352)
  Documentation improvements (pydata#3328)
  Remove `complex.nc` from built docs (pydata#3353)
  Fix DataArray.to_netcdf type annotation (pydata#3325)
  CI test suites with pinned minimum dependencies (pydata#3346)
  Bugfix/plot accept coord dim (pydata#3345)
  CI environments overhaul (pydata#3340)
  Add hypothesis support to related projects (pydata#3335)
  More doc fixes (pydata#3333)
  Improve the documentation of swap_dims (pydata#3331)
  fix the doc names of the return value of swap_dims (pydata#3329)
@keewis keewis mentioned this pull request Dec 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@keewis @dcherian