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

Rewrite installation instructions to make conda use clearer #1711

Merged
merged 3 commits into from Jun 2, 2021

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jun 1, 2021

We generally tell new users to use conda for installing Satpy. This PR rearranges the installation instructions so conda comes before pip and also adds information about miniconda.

@djhoese djhoese requested review from mraspaud and pnuu June 1, 2021 17:05
@djhoese djhoese self-assigned this Jun 1, 2021
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.

LGTM

@codecov
Copy link

codecov bot commented Jun 1, 2021

Codecov Report

Merging #1711 (7690dd3) into main (aab24d4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1711   +/-   ##
=======================================
  Coverage   92.43%   92.43%           
=======================================
  Files         258      258           
  Lines       38273    38273           
=======================================
  Hits        35376    35376           
  Misses       2897     2897           
Flag Coverage Δ
behaviourtests 4.84% <ø> (ø)
unittests 92.97% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 aab24d4...7690dd3. Read the comment docs.

@@ -47,7 +28,7 @@ you haven't created and activated one already, you can by running:

.. code-block:: bash

$ conda create -n my_satpy_env python
$ conda create -c conda-forge -n my_satpy_env python
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to advise to install satpy directly at env creation to make the installation faster? Or add a short comment like "It is also possible to define all the wanted packages in the environment creation command, which makes the installation faster and might solve some dependency issues between package versions."

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah good point. I'll see if I can restructure this.

Copy link
Member

@pnuu pnuu left a comment

Choose a reason for hiding this comment

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

LGTM. Just one comment.

@coveralls
Copy link

coveralls commented Jun 1, 2021

Coverage Status

Coverage remained the same at 92.919% when pulling 7690dd3 on djhoese:doc-install-reorder into aab24d4 on pytroll:main.

@djhoese
Copy link
Member Author

djhoese commented Jun 1, 2021

Not sure if the two note sections look bad in the "existing" section.

image

@mraspaud
Copy link
Member

mraspaud commented Jun 1, 2021

Could the second note link to the check_satpy utility?

@mraspaud
Copy link
Member

mraspaud commented Jun 1, 2021

And the notes are fine.

@djhoese
Copy link
Member Author

djhoese commented Jun 1, 2021

Could the second note link to the check_satpy utility?

Done.

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.

LGTM

@mraspaud mraspaud merged commit a50246a into pytroll:main Jun 2, 2021
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.

Reorder installation instructions to put conda before PyPI
4 participants