python312Packages.plotly: disable checks on darwin, unbreak #347567
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 tasks
|
@ofborg build python311Packages.plotly python312Packages.plotly |
|
During a nixpkgs-review on x86-64-Linux when building for python 3.11 Edit: also breaks in the CI, same error): > FAILED plotly/tests/test_optional/test_px/test_px_input.py::test_build_df_from_vaex_and_polars[vaex] - ModuleNotFoundError: No module named 'vaex'
> FAILED plotly/tests/test_optional/test_px/test_px_input.py::test_build_df_from_vaex_and_polars[polars] - ModuleNotFoundError: No module named 'polars'
> FAILED plotly/tests/test_optional/test_px/test_px_input.py::test_build_df_with_hover_data_from_vaex_and_polars[hover_data0-vaex] - ModuleNotFoundError: No module named 'vaex'
> FAILED plotly/tests/test_optional/test_px/test_px_input.py::test_build_df_with_hover_data_from_vaex_and_polars[hover_data0-polars] - ModuleNotFoundError: No module named 'polars'
> FAILED plotly/tests/test_optional/test_px/test_px_input.py::test_build_df_with_hover_data_from_vaex_and_polars[hover_data1-vaex] - ModuleNotFoundError: No module named 'vaex'
> FAILED plotly/tests/test_optional/test_px/test_px_input.py::test_build_df_with_hover_data_from_vaex_and_polars[hover_data1-polars] - ModuleNotFoundError: No module named 'polars' |
|
ah, i misread the logs while fixing #347564, i'll move the fix here |
|
|
|
likely oom, you either need to wait for ofborg eval or install more ram |
Pandapip1
approved these changes
Oct 9, 2024
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/2029 |
I've done that too. An easy mistake! P.S. Test error is fixed, just awaiting nixpkgs-review |
|
Result of 8 packages marked as broken and skipped:
122 packages built:
|
|
Result of 28 packages marked as broken and skipped:
21 packages failed to build:
71 packages built:
|
|
darwin derivation is now unchanged from before the checkphase was added. Can be addressed in a followup, less time-pressed PR |
|
seems to be a spike in ofborg eval queue, it might take a while |
sarahec
approved these changes
Oct 9, 2024
It looks like |
sarahec
suggested changes
Oct 10, 2024
| @@ -1,5 +1,6 @@ | |||
| { | |||
| lib, | |||
| stdenv, | |||
Comment on lines
+74
to
+76
| # the check inputs are broken on darwin | ||
| doCheck = !stdenv.hostPlatform.isDarwin; | ||
|
|
There was a problem hiding this comment.
Suggested change
| # the check inputs are broken on darwin | |
| doCheck = !stdenv.hostPlatform.isDarwin; | |
| optional-dependencies = { | |
| orca = [ orca ]; | |
| }; |
and change nativeCheckInputs to checkInputs
| @@ -116,6 +120,9 @@ buildPythonPackage rec { | |||
| "test_dependencies_not_imported" | |||
| # FAILED test_init/test_lazy_imports.py::test_lazy_imports - AssertionError: assert 'plotly' not in {'IPython': <module 'IPython' from '... | |||
| "test_lazy_imports" | |||
| # requires vaex and polars, vaex is not packaged | |||
| "test_build_df_from_vaex_and_polars" | |||
| "test_build_df_with_hover_data_from_vaex_and_polars" | |||
There was a problem hiding this comment.
Suggested change
| "test_build_df_with_hover_data_from_vaex_and_polars" | |
| "test_build_df_with_hover_data_from_vaex_and_polars" | |
| # Cannot launch kaleido as a subprocess. This would need a passthru test. | |
| "test_kaleido_engine_to_image_returns_bytes" | |
| "test_kaleido_fulljson" | |
| "test_bytesio" | |
| "test_png_renderer_mimetype" | |
| "test_svg_renderer_show" | |
| "test_pdf_renderer_show_override" | |
| "test_mimetype_combination" | |
| # Cannot run on systes without orca | |
| "test_scraper" | |
| "test_bytesio" |
There was a problem hiding this comment.
How about
Suggested change
| "test_build_df_with_hover_data_from_vaex_and_polars" | |
| "test_build_df_with_hover_data_from_vaex_and_polars" | |
| ] ++ lib.optionals stdenv.hostPlatform.isDarwin [ | |
| # Cannot launch kaleido as a subprocess. This would need a passthru test. | |
| "test_kaleido_engine_to_image_returns_bytes" | |
| "test_kaleido_fulljson" | |
| "test_bytesio" | |
| "test_png_renderer_mimetype" | |
| "test_svg_renderer_show" | |
| "test_pdf_renderer_show_override" | |
| "test_mimetype_combination" | |
| # Cannot run on systes without orca | |
| "test_scraper" | |
| "test_bytesio" | |
| ]; |
There was a problem hiding this comment.
@Pandapip1 that's even better.
|
I would argue the important thing is getting this merged to fix the broken builds; 11-100 additional rebuilds aren't worth blocking this PR. As per usual, the aarch64-darwin tests have stalled; this PR should be ready for merge. |
This was referenced Oct 10, 2024
|
I got it to build by simply disabling those two tests: https://github.com/NixOS/nixpkgs/pull/347732/files#diff-efb1e886a286b10961f06040b7b538e22155f85984a2098f190c9751bbb63f7fR80-R82 |
On Darwin? Sandboxed? |
It looks like my change did not affect darwin platforms for some reason, so I don't know. |
|
darwin was broken since some check inputs dont eval on darwin, disabling checkPhase makes it so the nativeCheckInputs are not passed to the derivation |
|
In any case, I have no objection to this getting merged as is plus a follow-up patch. Or the fixes can be added (easily) and this is done the right way. |
|
i held of my own cleanup fix due to the slow eval, i'll open a followup |
|
please view #347763 |
13 tasks
|
It was broken on Darwin for all Python versions. Edit: Whoops, I see you've already responded. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.