Skip to content

Conversation

@gerritholl
Copy link
Member

In the docstring for Scene.__init__, rephrase the code example that was referring to Dataset. Use xarray.DataArray instead.

  • Closes #xxxx
  • Tests added
  • Tests passed
  • Passes flake8 satpy
  • Fully documented

In the docstring for `Scene.__init__`, rephrase the code example that was referring to `Dataset`.  Use `xarray.DataArray` instead.
@ghost
Copy link

ghost commented Apr 15, 2020

Congratulations 🎉. DeepCode analyzed your code in 2.077 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably have a Satpy-wide docstring review since the word Dataset probably appears a lot (sometimes referring to the xarray object, sometimes the legacy satpy object).

Could you rename the title of this PR to something that would work in the release notes? Our release process takes issue/PR titles and puts them in the change log.

@djhoese djhoese self-assigned this Apr 15, 2020
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 89.605% when pulling e5a1011 on gerritholl:patch-3 into 446af3a on pytroll:master.

@codecov
Copy link

codecov bot commented Apr 15, 2020

Codecov Report

Merging #1146 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1146      +/-   ##
==========================================
- Coverage   89.60%   89.60%   -0.01%     
==========================================
  Files         200      200              
  Lines       29484    29484              
==========================================
- Hits        26420    26419       -1     
- Misses       3064     3065       +1     
Impacted Files Coverage Δ
satpy/scene.py 90.01% <ø> (-0.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 446af3a...e5a1011. Read the comment docs.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just some adjustment needed. I also agree with @djhoese about changing the title of the PR to something like Replace a mention of Dataset with DataArray in the documentation.


scn = Scene()
scn['my_dataset'] = Dataset(my_data_array, **my_info)
scn['my_dataset'] = xr.DataArray(my_data_array, **my_info)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
scn['my_dataset'] = xr.DataArray(my_data_array, **my_info)
scn['my_dataset'] = xr.DataArray(my_data_array, attrs=my_metadata)

my_info is supposed to be metadata.

then the available readers will be searched for a Reader that can support the provided files. This can take
a considerable amount of time so it is recommended that `reader` always be provided. Note without `filenames`
the Scene is created with no Readers available requiring Datasets to be added manually::
the Scene is created with no Readers available requiring DataArrays to be added manually::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you make this link to the xarray api documentation ?

@gerritholl gerritholl marked this pull request as draft May 18, 2020 09:41
@ameraner
Copy link
Member

A user just bumped into this issue, saying that "creating a Dataset doesn't work" :)

@djhoese
Copy link
Member

djhoese commented Jun 21, 2023

Just want to mention I've been using "product" as the generic term for DataArrays used in Satpy, especially when talking to people unfamiliar with xarray/python. Could be useful as an alternative to "dataset" for docstrings and documentation where we want it to be non-programmer friendly but also not confuse programmers familiar with xarray.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants