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

override: fix pyarrow #1738

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Conversation

TyberiusPrime
Copy link
Contributor

[ ] There's an automated test for this change

  • Commit messages or code include references to related issues or PRs (including third parties)
  • Commit messages are conventional - examples from the log include "feat: add changelog files to fixup hook", "fix(contourpy): allow wheel usage", and "test: add sqlalchemy2 test"

I have trouble building anything with pyarrow against nixpkgs 24.05. Might be a change in lib.versions.majorMinor?

Anyway, this fix should work in both new and old.

@@ -2241,8 +2241,8 @@ lib.composeManyExtensions [
);

ARROW_HOME = _arrow-cpp;
arrowCppVersion = lib.versions.majorMinor _arrow-cpp;
pyArrowVersion = lib.versions.majorMinor prev.pyarrow;
arrowCppVersion = lib.versions.majorMinor (_arrow-cpp.version or _arrow-cpp);
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what this is supposed to do.
The or case would pass the derivation to majorMinor, not the version string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, passing the derivation is what the current override apparently does
See

arrowCppVersion = lib.versions.majorMinor _arrow-cpp;

But then, I don't think majorminor ever took a derivation.
So I assumed that in the past, it must have been a version string.

I figured I'd code it defensively, if it has a .version, use that, and if it doesn't it must be the version string to actually work with majorminor, so pass that.

But there's of course a good chance that I misunderstood something about the original code and the nixpgks it lived in.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am also hitting this error right now. I think this code never worked in the past. Just change both lines to .version. Backwards compatibility is not needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@TyberiusPrime I would really love to see this merged. Can you remove the or and just use .version? I think this PR would be ready to go then.

@usmcamp0811
Copy link

I just used this PR to try and make my example-flink-job usable on MacOS and things did not work. Some context is that apache-flink has pyarrow as a dependency. After switching out the import in my flake.nix I had my buddy attempt to run my package with his Mac

nix run gitlab:usmcamp0811/dotfiles/ed1c16132787a5bb048de61249420962513a50f3#example-flink-job.python

and he got this error:

nix run --refresh gitlab:usmcamp0811/dotfiles#example-flink-job.python

error:
       … while calling the 'derivationStrict' builtin

         at /derivation-internal.nix:9:12:

            8|
            9|   strict = derivationStrict drvAttrs;
             |            ^
           10|while evaluating derivation 'python3-3.11.9-env'
         whose name attribute is located at /nix/store/sj9yrq21wbbfr5715hys3laa2qd6x471-source/pkgs/stdenv/generic/make-derivation.nix:333:7

       … while evaluating attribute 'passAsFile' of derivation 'python3-3.11.9-env'

         at /nix/store/sj9yrq21wbbfr5715hys3laa2qd6x471-source/pkgs/build-support/trivial-builders/default.nix:69:9:

           68|         inherit buildCommand name;
           69|         passAsFile = [ "buildCommand" ]
             |         ^
           70|           ++ (derivationArgs.passAsFile or [ ]);

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: arrow-cpp version (16.0) mismatches pyarrow version (11.0)

apache-flink relies on apache-beam which seems to be locked to version 11.0 I tried to update pyarrow but got all this from Poetry.

╰─🚧 poetry add pyarrow@16.0


Updating dependencies
Resolving dependencies... (0.3s)

    Because apache-flink (1.19.1) depends on apache-beam (>=2.43.0,<2.49.0)
 and no versions of apache-flink match >1.19.1,<1.20.0 || >1.20.0,<2.0.0, apache-flink (>=1.19.1,<1.20.0 || >1.20.0,<2.0.0) requires apache-beam (>=2.43.0,<2.49.0).
(1) So, because apache-flink (1.20.0) depends on apache-beam (>=2.43.0,<2.49.0), apache-flink (>=1.19.1,<2.0.0) requires apache-beam (>=2.43.0,<2.49.0).

    Because no versions of apache-beam match >2.43.0,<2.44.0 || >2.44.0,<2.45.0 || >2.45.0,<2.46.0 || >2.46.0,<2.47.0 || >2.47.0,<2.48.0 || >2.48.0,<2.49.0
 and apache-beam (2.43.0) depends on pyarrow (>=0.15.1,<10.0.0), apache-beam (>=2.43.0,<2.44.0 || >2.44.0,<2.45.0 || >2.45.0,<2.46.0 || >2.46.0,<2.47.0 || >2.47.0,<2.48.0 || >2.48.0,<2.49.0) requires pyarrow (>=0.15.1,<10.0.0).
    And because apache-beam (2.44.0) depends on pyarrow (>=0.15.1,<10.0.0), apache-beam (>=2.43.0,<2.45.0 || >2.45.0,<2.46.0 || >2.46.0,<2.47.0 || >2.47.0,<2.48.0 || >2.48.0,<2.49.0) requires pyarrow (>=0.15.1,<10.0.0).
    And because apache-beam (2.45.0) depends on pyarrow (>=0.15.1,<10.0.0)
 and apache-beam (2.46.0) depends on pyarrow (>=3.0.0,<10.0.0), apache-beam (>=2.43.0,<2.47.0 || >2.47.0,<2.48.0 || >2.48.0,<2.49.0) requires pyarrow (>=0.15.1,<10.0.0).
    And because apache-beam (2.47.0) depends on pyarrow (>=3.0.0,<12.0.0)
 and apache-beam (2.48.0) depends on pyarrow (>=3.0.0,<12.0.0), apache-beam (>=2.43.0,<2.49.0) requires pyarrow (>=0.15.1,<12.0.0).
    And because apache-flink (>=1.19.1,<2.0.0) requires apache-beam (>=2.43.0,<2.49.0) (1), apache-flink (>=1.19.1,<2.0.0) requires pyarrow (>=0.15.1,<12.0.0)
    So, because example-flink-job depends on both apache-flink (^1.19.1) and pyarrow (16.0), version solving failed.

otherwise this ran fine on all my Linux boxes...

@TyberiusPrime
Copy link
Contributor Author

   error: arrow-cpp version (16.0) mismatches pyarrow version (11.0)

The fact that you even get that error message shows that the fix did work.

You probably will have to use a wheel though if you need a pyarrow that does not match nixpkgs' arrow-cpp.

@TyberiusPrime
Copy link
Contributor Author

Removed the defensive coding as requested.

@m1-s
Copy link
Contributor

m1-s commented Sep 1, 2024

@adisbladis Can you take another look here?

@cpcloud cpcloud merged commit 6d8507d into nix-community:master Sep 27, 2024
179 checks passed
@bouk bouk mentioned this pull request Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants