fetchPypi -> fetchFromPyPI #146564

AndersonTorres · 2021-11-19T00:23:25Z

Motivation for this change

Since the Python Package Index works as a package index, it is more intuitive to
call the function that fetches packages from it fetchFromPyPI, following the
unwritten rule for sources obtained from a "repository" like fetchFrom{GitHub,
Gitlab, Bitbucket, ...}

For now, this commit will act as a placeholder for the new fetchFromPyPI
function, while the old one will act as a safe alias.

Things done

jonringer

I can't think of any reasons not to, and it's more consistent with the other platform-specific fetchers.

curious if @FRidh has any feedback though.

AndersonTorres · 2021-11-19T01:30:28Z

#119896

figsoda · 2021-11-19T02:04:15Z

why PyPI instead of Pypi?

AndersonTorres · 2021-11-19T02:51:07Z

why PyPI instead of Pypi?

Py thon Package Index

figsoda · 2021-11-19T03:58:28Z

I am not sure about the benefits of capitalizing acronyms in here
It is slightly harder to type and makes case conversions harder, especially when there is a lower case letter within the acronym

pkgs/development/interpreters/python/update-python-libraries/update-python-libraries.py

figsoda · 2021-11-19T04:00:55Z

pkgs/top-level/python-packages.nix

@@ -56,7 +56,9 @@ let
  # See build-setupcfg/default.nix for documentation.
  buildSetupcfg = import ../build-support/build-setupcfg self;

-  fetchPypi = callPackage ../development/interpreters/python/fetchpypi.nix { };
+  # fetchPypi -> fetchFromPyPI
+  fetchPypi = fetchFromPyPI;


maybe we can deprecate this and remove in the (long) future?

there's not a good way to deprecate within nixpkgs. If you emit warnings then that breaks a lot of CI's (including ofborg)

If you emit warnings then that breaks a lot of CI's (including ofborg)

Which is kinda stupid.

I see myself already doing this quite often so why not.

Suggested change

fetchPypi = fetchFromPyPI;

fetchPypi = fetchFromPyPI;

fetchFromPyPi = throw "Did you mean fetchFromPyPI?"

Is this useful? The compiler will catch it anyway...

I would say so, it's pretty common to do a typo like this, but it reads as correct.

Is this useful? The compiler will catch it anyway...

I am all about this alias. Same as fetchFromGithub.

It looks like a case for a whole mistaken-aliases.nix file.

legendofmiracles · 2021-11-19T05:14:35Z

I am not sure about the benefits of capitalizing acronyms in here

It is slightly harder to type and makes case conversions harder, especially when there is a lower case letter within the acronym

We also have mixed case with the GitHub fetcher.
Sure, I guess there's a difference between an acronym and the real name of a platform, but generally PyPI also always seems to be capitalized.

SuperSandro2000

Using the new fetcher will probably break nix-update, nixpkgs-update and nix-prefetch-url.

SuperSandro2000 · 2021-11-19T14:30:43Z

pkgs/top-level/python-packages.nix

@@ -56,7 +56,9 @@ let
  # See build-setupcfg/default.nix for documentation.
  buildSetupcfg = import ../build-support/build-setupcfg self;

-  fetchPypi = callPackage ../development/interpreters/python/fetchpypi.nix { };
+  # fetchPypi -> fetchFromPyPI


Suggested change

# fetchPypi -> fetchFromPyPI

This is pretty clear from the line below, no need to comment it.

Using the new fetcher will probably break nix-update, nixpkgs-update and nix-prefetch-url.

How?
This is not a "new fetcher", it is the same with a new name. Do the updaters rely on the name of the function?

This is pretty clear from the line below, no need to comment it.

Okay!

nix-prefetch-url should be fine. AFAIK, it only works with the nix store and curl.

nix-update might be sensitive to that term. But it's not like we are rolling it out immediately

Sorry I accidently edited your message instead of replying @SuperSandro2000

I wanted to say that there are other possible spellings like fetchFromPypi, I'm not sure about the change in capitalization

SuperSandro2000 · 2021-11-19T14:31:37Z

pkgs/top-level/python-packages.nix

@@ -56,7 +56,9 @@ let
  # See build-setupcfg/default.nix for documentation.
  buildSetupcfg = import ../build-support/build-setupcfg self;

-  fetchPypi = callPackage ../development/interpreters/python/fetchpypi.nix { };
+  # fetchPypi -> fetchFromPyPI
+  fetchPypi = fetchFromPyPI;


If you emit warnings then that breaks a lot of CI's (including ofborg)

Which is kinda stupid.

I see myself already doing this quite often so why not.

Suggested change

fetchPypi = fetchFromPyPI;

fetchPypi = fetchFromPyPI;

fetchFromPyPi = throw "Did you mean fetchFromPyPI?"

Since the Python Package Index works as a package index, it is more intuitive to call the function that fetches packages from it `fetchFromPyPI`, following the unwritten rule for sources obtained from a "repository" like fetchFrom{GitHub, Gitlab, Bitbucket, ...} For now, this commit will act as a placeholder for the new fetchFromPyPI function, while the old one will act as a safe alias.

AndersonTorres · 2021-11-19T20:42:09Z

I have tested it locally. It is failing:

error: attribute 'fetchFromPyPI' missing

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/pkgs/development/interpreters/python/default.nix:37:25:

           36|           };
           37|           keep = self: {
             |                         ^
           38|             # TODO maybe only define these here so nothing is needed to be kept in sync.

       … while evaluating the attribute 'src.name'

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/pkgs/development/python-modules/3to2/default.nix:11:3:

           10|
           11|   src = fetchFromPyPI {
             |   ^
           12|     inherit version;

       … while evaluating 'hasSuffix'

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/lib/strings.nix:234:5:

          233|     # Input string
          234|     content:
             |     ^
          235|     let

       … from call site

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/pkgs/development/interpreters/python/mk-python-derivation.nix:127:25:

          126|       pythonRemoveBinBytecodeHook
          127|     ] ++ lib.optionals (lib.hasSuffix "zip" (attrs.src.name or "")) [
             |                         ^
          128|       unzip

       … while evaluating 'optionals'

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/lib/lists.nix:270:5:

          269|     # List to return if condition is true
          270|     elems: if cond then elems else [];
             |     ^
          271|

       … from call site

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/pkgs/development/interpreters/python/mk-python-derivation.nix:127:10:

          126|       pythonRemoveBinBytecodeHook
          127|     ] ++ lib.optionals (lib.hasSuffix "zip" (attrs.src.name or "")) [
             |          ^
          128|       unzip

       … while evaluating 'chooseDevOutputs'

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/lib/attrsets.nix:500:22:

          499|   /* Pick the outputs of packages to place in buildInputs */
          500|   chooseDevOutputs = drvs: builtins.map getDev drvs;
             |                      ^
          501|

       … from call site

       … while evaluating the attribute 'nativeBuildInputs' of the derivation 'python2.7-py3to2-1.1.1'

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/pkgs/stdenv/generic/make-derivation.nix:205:7:

          204|     // (lib.optionalAttrs (attrs ? name || (attrs ? pname && attrs ? version)) {
          205|       name =
             |       ^
          206|         let

       … while evaluating the attribute 'drvPath'

       at /nix/store/rplkxyhnzsz7cj435ppifx2jlmv9jipj-source/lib/customisation.nix:163:7:

          162|     in commonAttrs // {
          163|       drvPath = assert condition; drv.drvPath;
             |       ^
          164|       outPath = assert condition; drv.outPath;

AndersonTorres · 2022-01-16T01:24:38Z

I will close and redo this.

FRidh · 2022-01-16T09:07:37Z

I have quite mixed feelings about this. By now, I don't really like fetchers that build upon other actual fetchers. So, in case of fetchPypi, I much rather have a function that returns the url that can then be passed to fetchurl. But I guess preferences differ. Without really having a documented way on what direction we want to go with fetchers I'd say either way could go.

AndersonTorres · 2022-01-16T14:45:36Z

I much rather have a function that returns the url that can then be passed to fetchurl.

Something like fetchurl {url = generatePyPIUrl {...}}?

AndersonTorres requested review from FRidh and jonringer as code owners November 19, 2021 00:23

github-actions bot added the 6.topic: python label Nov 19, 2021

jonringer approved these changes Nov 19, 2021

View reviewed changes

figsoda reviewed Nov 19, 2021

View reviewed changes

pkgs/development/interpreters/python/update-python-libraries/update-python-libraries.py Outdated Show resolved Hide resolved

figsoda reviewed Nov 19, 2021

View reviewed changes

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Nov 19, 2021

SuperSandro2000 reviewed Nov 19, 2021

View reviewed changes

AndersonTorres added 3 commits November 19, 2021 17:29

update-python-libraries.py: fetchPypi -> fetchFromPyPI

f2eda86

[wip] pythonPackages.py3to2: fetchPypi -> fetchFromPyPI

2df13b1

AndersonTorres requested review from jonringer and thiagokokada November 30, 2021 19:56

AndersonTorres closed this Jan 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fetchPypi -> fetchFromPyPI #146564

fetchPypi -> fetchFromPyPI #146564

AndersonTorres commented Nov 19, 2021 •

edited

Loading

jonringer left a comment

AndersonTorres commented Nov 19, 2021

figsoda commented Nov 19, 2021

AndersonTorres commented Nov 19, 2021

figsoda commented Nov 19, 2021

figsoda Nov 19, 2021

jonringer Nov 19, 2021

SuperSandro2000 Nov 19, 2021 •

edited by figsoda

Loading

AndersonTorres Nov 19, 2021

jonringer Nov 19, 2021

thiagokokada Nov 20, 2021

AndersonTorres Nov 20, 2021

legendofmiracles commented Nov 19, 2021

SuperSandro2000 left a comment

SuperSandro2000 Nov 19, 2021 •

edited

Loading

AndersonTorres Nov 19, 2021

jonringer Nov 19, 2021

figsoda Nov 19, 2021

SuperSandro2000 Nov 19, 2021 •

edited by figsoda

Loading

AndersonTorres commented Nov 19, 2021

AndersonTorres commented Jan 16, 2022

FRidh commented Jan 16, 2022

AndersonTorres commented Jan 16, 2022

	fetchPypi = fetchFromPyPI;
	fetchPypi = fetchFromPyPI;
	fetchFromPyPi = throw "Did you mean fetchFromPyPI?"

fetchPypi -> fetchFromPyPI #146564

fetchPypi -> fetchFromPyPI #146564

Conversation

AndersonTorres commented Nov 19, 2021 • edited Loading

Motivation for this change

Things done

jonringer left a comment

Choose a reason for hiding this comment

AndersonTorres commented Nov 19, 2021

figsoda commented Nov 19, 2021

AndersonTorres commented Nov 19, 2021

figsoda commented Nov 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuperSandro2000 Nov 19, 2021 • edited by figsoda Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

legendofmiracles commented Nov 19, 2021

SuperSandro2000 left a comment

Choose a reason for hiding this comment

SuperSandro2000 Nov 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuperSandro2000 Nov 19, 2021 • edited by figsoda Loading

Choose a reason for hiding this comment

AndersonTorres commented Nov 19, 2021

AndersonTorres commented Jan 16, 2022

FRidh commented Jan 16, 2022

AndersonTorres commented Jan 16, 2022

AndersonTorres commented Nov 19, 2021 •

edited

Loading

SuperSandro2000 Nov 19, 2021 •

edited by figsoda

Loading

SuperSandro2000 Nov 19, 2021 •

edited

Loading

SuperSandro2000 Nov 19, 2021 •

edited by figsoda

Loading