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

doc: Rewrite python onboarding tutorials in manual #87450

Merged
merged 1 commit into from May 15, 2020

Conversation

bhipple
Copy link
Contributor

@bhipple bhipple commented May 10, 2020

Based on some feedback in #87094 and discussion with @FRidh, this re-organizes
the onboarding tutorial in the Nixpkgs manual's python section, so that we start
with the simplest, most ad-hoc examples and work our way up. This progresses
from:

  1. How to create an temporary python env at the cmdline, then
  2. How to create a specific python env for a single script, then
  3. How to create a specific python env for a project in a shell.nix, then
  4. How to install a specific python env globally on the system or in a user profile.

Additionally, I've tried to standardize on some of the "best practice" ways of
doing things:

  1. Instead of saying that this command style is "supported but strongly
    discouraged", I've just deleted it to avoid confusion.

    Bad: nix-shell -p python38Packages.numpy python38Packages.toolz
    Good: nix-shell -p 'python38.withPackages(ps: with ps; [ numpy toolz ])'

  2. In the portion where we show how to add stuff to the user's
    XDG_CONFIG_HOME, use overlays instead of config.nix. The former can do
    everything the latter can do, but is also much more generic and powerful,
    because it can compose with other files, compose with other envs, compose
    with overlays that do things like swap whether tensorflow and pytorch are
    built openblas/mkl/cuda stacks, and so on. The user is eventually going to
    see the overlay, so to avoid confusion let's standardize on it.

Let me know what you guys think!

Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

This looks good to me, thank you for picking this up! Often there are remarks on documentation but it is not often someone actually does something about it.

I only have minor comments. One small change I would like to see is that "Python" is used throughout the text instead of "python".

doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved
doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved
doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved
doc/languages-frameworks/python.section.md Show resolved Hide resolved
doc/languages-frameworks/python.section.md Show resolved Hide resolved
doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved
doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved
(`python setup.py develop`); instead of installing the package this command
creates a special link to the project code. That way, you can run updated code
without having to reinstall after each and every change you make. Development
mode is also available. Let's see how you can use it.
Copy link
Member

Choose a reason for hiding this comment

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

is also available for setuptools-based projects.

While we now have PEP 517/518, there is no generic approach (yet).

@bhipple
Copy link
Contributor Author

bhipple commented May 10, 2020

(While we iterate on the reviews, I'll leave my commits unsquashed so you can read incremental diffs, but once we get something we're happy with I'll rebase/cleanup the git history prior to merging.)

I've updated with the comments. As alluded to in #87440 (comment), I think there's some work to be done on the lower half of the manual as well, particularly around how to handle overlays and the examples involved. Right now it feels a little convoluted and I'd like to highlight the most standard use cases first; but ideally I'd like to do that in a follow-up PR focused on that topic and keep this one focused on the new user onboarding tutorial. In the meantime I've fixed the blatant error on numpy's version, but it's not a particularly useful example at the moment.

```nix
with import <nixpkgs> {};
Applications on Nix are typically installed into your user profile imperatively
using `nix-env -i`, and on NixOS declaratively by adding the package name to
Copy link
Member

Choose a reason for hiding this comment

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

(nitpick) I'd use nix-env -iA here, especially beginners shouldn't even get used to -i-only IMHO.

Copy link
Member

Choose a reason for hiding this comment

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

If a file does not contain an attribute set, but only a top-level derivation you need to use nix-env -i -f ....

It takes an argument `buildPythonPackage`.
We now call this function using `callPackage` in the definition of our environment
It takes an argument `buildPythonPackage`. We now call this function using
`callPackage` in the definition of our environment
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we suggest python3Packages.callPackage here?

doc/languages-frameworks/python.section.md Show resolved Hide resolved
Based on some feedback in NixOS#87094 and discussion with @FRidh, this re-organizes
the onboarding tutorial in the Nixpkgs manual's python section, so that we start
with the simplest, most ad-hoc examples and work our way up. This progresses
from:

1. How to create an temporary python env at the cmdline, then
2. How to create a specific python env for a single script, then
3. How to create a specific python env for a project in a shell.nix, then
4. How to install a specific python env globally on the system or in a user profile.

Additionally, I've tried to standardize on some of the "best practice" ways of
doing things:

1. Instead of saying that this command style is "supported but strongly not
   discouraged", I've just deleted it to avoid confusion.

   Bad:  nix-shell -p python38Packages.numpy python38Packages.toolz
   Good: nix-shell -p 'python38.withPackages(ps: with ps; [ numpy toolz ])'

2. In the portion where we show how to add stuff to the user's
   `XDG_CONFIG_HOME`, use overlays instead of `config.nix`. The former can do
   everything the latter can do, but is also much more generic and powerful,
   because it can compose with other files, compose with other envs, compose
   with overlays that do things like swap whether tensorflow and pytorch are
   built openblas/mkl/cuda stacks, and so on. The user is eventually going to
   see the overlay, so to avoid confusion let's standardize on it.
@bhipple
Copy link
Contributor Author

bhipple commented May 14, 2020

I think this is a shippable improvement over the status quo, so I cleaned up the git history and propose merging. Not sure if I'll have time for a few days, but in a follow up I think we can make further improvements around the bottom section regarding overlays.

@jonringer
Copy link
Contributor

I took a slightly different approach in my python video: https://www.youtube.com/watch?v=jXd-hkP4xnU&lc=Ugxi6MkvJXqv0Be2mtx4AaABAg and just used the raw python packages.

But your changes LGTM :)

@FRidh FRidh merged commit c882907 into NixOS:master May 15, 2020
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

5 participants