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 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!

@bhipple bhipple requested a review from FRidh as a code owner May 10, 2020
@bhipple bhipple force-pushed the bhipple:doc/py-tutorial branch from f0535f1 to 8e58c6b May 10, 2020
@bhipple bhipple requested a review from jonringer May 10, 2020
Copy link
Member

FRidh left a comment

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 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.

This comment has been minimized.

Copy link
@FRidh

FRidh May 10, 2020

Member

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

This comment has been minimized.

Copy link
@Ma27

Ma27 May 11, 2020

Member

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

This comment has been minimized.

Copy link
@FRidh

FRidh May 11, 2020

Member

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

This comment has been minimized.

Copy link
@Ma27

Ma27 May 11, 2020

Member

shouldn't we suggest python3Packages.callPackage here?

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 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 bhipple force-pushed the bhipple:doc/py-tutorial branch from a2ea935 to a9f3b31 May 14, 2020
@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

jonringer commented May 14, 2020

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
13 checks passed
13 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a9f3b31"; rev="a9f3b31166528361758e6815eef768531cce19f9"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a9f3b31"; rev="a9f3b31166528361758e6815eef768531cce19f9"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a9f3b31"; rev="a9f3b31166528361758e6815eef768531cce19f9"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a9f3b31"; rev="a9f3b31166528361758e6815eef768531cce19f9"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a9f3b31"; rev="a9f3b31166528361758e6815eef768531cce19f9"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a9f3b31"; rev="a9f3b31166528361758e6815eef768531cce19f9"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="a9f3b31"; rev="a9f3b31166528361758e6815eef768531cce19f9"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.