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.

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

Add example to the documentation using multiple readers #2071

Conversation

adybbroe
Copy link
Contributor

@adybbroe adybbroe commented Mar 24, 2022

Just giving an example of providing a dataset that requires multiple readers.

It was quite clear from the documentation already, but thought it was nice giving an explicit example, as at least I have been in doubt a couple of times how to do it correct.

  • Closes #xxxx
  • Tests added
  • Fully documented
  • Add your name to AUTHORS.md if not there already

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>
@adybbroe adybbroe marked this pull request as ready for review March 25, 2022 13:34
@adybbroe
Copy link
Contributor Author

I thought RTD pages should be build for the PR, so I could check it? Wasn't that the case previously? Or am I thinking of the Pyspectral repo?

Copy link
Collaborator

@gerritholl gerritholl left a comment

Choose a reason for hiding this comment

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

Just a few typographic corrections.

satpy/scene.py Outdated Show resolved Hide resolved
satpy/scene.py Outdated Show resolved Hide resolved
satpy/scene.py Outdated Show resolved Hide resolved
adybbroe and others added 3 commits March 25, 2022 14:53
Fix typographic error

Co-authored-by: Gerrit Holl <gerrit.holl@gmail.com>
Fix typo

Co-authored-by: Gerrit Holl <gerrit.holl@gmail.com>
Fix typo

Co-authored-by: Gerrit Holl <gerrit.holl@gmail.com>
@adybbroe
Copy link
Contributor Author

Just a few typographic corrections.

Thanks @gerritholl ! That was sloppy of me, good you saw it.

@adybbroe
Copy link
Contributor Author

Anything you are okay with @djhoese and @mraspaud and you think we can merge?
Not that it is in a hurry of course - it is just a tiny PR, so can also be forgotten easily...

satpy/scene.py Outdated Show resolved Hide resolved
satpy/scene.py Outdated Show resolved Hide resolved
@djhoese
Copy link
Member

djhoese commented Mar 30, 2022

I thought RTD pages should be build for the PR, so I could check it? Wasn't that the case previously? Or am I thinking of the Pyspectral repo?

I think you said this last time @adybbroe 😉 I don't remember RTD's current preferred way of requesting PRs to be built is. We do have a CI job that builds the docs which is at least a first step in catching errors. I don't remember if I made it make an "artifact" in the CI so that you can download the site and see the results. That of course could/should be replaced by RTD generation if it doesn't cost us anything.

…se comes last

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>
satpy/scene.py Outdated
Comment on lines 91 to 92
``filenames`` needs to be a `dict` (see parameters list below), like
e.g.::
Copy link
Member

Choose a reason for hiding this comment

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

The like might be redundant or unnecessary here. It reads a little funny.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. Done.

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.

LGTM thanks for making the changes.

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>
@adybbroe adybbroe self-assigned this Apr 2, 2022
@adybbroe adybbroe merged commit c911333 into pytroll:main Apr 4, 2022
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.

None yet

3 participants