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

marimo: 0.9.1 -> 0.9.9 #348012

Closed
wants to merge 2 commits into from
Closed

Conversation

dmadisetti
Copy link
Contributor

@dmadisetti dmadisetti commented Oct 11, 2024

  • Added narwhals at 1.9.7

  • Updated marimo build system to hatch + deps + version

  • Removed tests because it was a noop (tests are not included in pypi)

  • Built on platform(s)

    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:

  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage

  • Tested basic functionality of all binary files (usually in ./result/bin/)

  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)

    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.


Add a 👍 reaction to pull requests you find important.

@nix-owners nix-owners bot requested a review from natsukium October 11, 2024 22:56
@dmadisetti dmadisetti added 8.has: package (new) This PR adds a new package 8.has: package (update) This PR updates a package to a newer version 11.by: package-maintainer This PR was created by the maintainer of the package it changes labels Oct 11, 2024
@ofborg ofborg bot requested a review from akshayka October 12, 2024 05:19
@dmadisetti dmadisetti changed the title marimo: 0.9.1 -> 0.9.7 marimo: 0.9.1 -> 0.9.9 Oct 14, 2024
@melvyn2
Copy link
Contributor

melvyn2 commented Oct 14, 2024

error: builder for '/nix/store/44myag68l3h65nml7i1vn9mvz1cq62dh-python3.12-pymdown-extensions-10.8.1.drv' failed with exit code 1;
       last 10 log lines:
       >     self.assertTrue(not diff)
       > E   AssertionError: False is not true
       > ----------------------------- Captured stdout call -----------------------------
       > --- Expected
       > +++ Actual
       > @@ -1 +1 @@
       > -<p><img alt="picture" src="file:///C%3A/Some/fake/path/extensions/_assets/bg.png" /></p>+<p><img alt="picture" src="file:C%3A/Some/fake/path/extensions/_assets/bg.png" /></p>
       > =========================== short test summary info ============================
       > FAILED tests/test_extensions/test_pathconverter.py::TestWindowsAbsFileScheme::test_windows_root_conversion - AssertionError: False is not true
       > ================= 1 failed, 530 passed, 1 deselected in 5.23s ==================
       For full logs, run 'nix-store -l /nix/store/44myag68l3h65nml7i1vn9mvz1cq62dh-python3.12-pymdown-extensions-10.8.1.drv'.
error: 1 dependencies of derivation '/nix/store/7kib346bxz8l0yips8rl9hyyjidb458d-python3.12-marimo-0.9.9.drv' failed to build
error: 1 dependencies of derivation '/nix/store/zkcix562w79fspdmab31987xv2v145mg-review-shell.drv' failed to build

2 packages failed to build:
marimo marimo.dist

6 packages built:
python311Packages.marimo python311Packages.marimo.dist python311Packages.narwhals python311Packages.narwhals.dist python312Packages.narwhals python312Packages.narwhals.dist

@dmadisetti
Copy link
Contributor Author

Thanks, was a little behind head.
pymdown has an outstanding PR: #348269
and this builds after patch

];

pythonImportsCheck = [ "narwhals" ];

Copy link
Member

Choose a reason for hiding this comment

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

Could you enable tests with pytestCheckHook?

hash = "sha256-Wz9SwCqGcwphFJfm/7rMqj34b8JkcMI+P7QNwrx5Prs=";
};

nativeBuildInputs = [
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nativeBuildInputs = [
build-system = [

};

build-system = [ setuptools ];
nativeBuildInputs = [ hatchling ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nativeBuildInputs = [ hatchling ];
build-system = [ hatchling ];

hatchling,
}:
let
pname = "narwhals";
Copy link
Member

Choose a reason for hiding this comment

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

narwhals is in already in staging (6432b6e).

Copy link
Member

Choose a reason for hiding this comment

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

This PR now conflicts with master

];

nativeCheckInputs = [ pytestCheckHook ];
Copy link
Member

Choose a reason for hiding this comment

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

If the tests are exclude then an option would be to use the source from GitHub. Persuade upstream to include the tests in PyPI releases usually takes some time.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 1, 2024
@Scrumplex Scrumplex mentioned this pull request Nov 1, 2024
13 tasks
@GaetanLepage
Copy link
Contributor

GaetanLepage commented Nov 1, 2024

I missed this PR and opened #353022.

  • I tried using the github source (which contains tests) but most of them fail (most probably because of network access)
  • Also, some static assets seem to only be available in the pypi archive
  • I explicitly disabled the tests (doCheck = false)

Feel free to take my changes if you want, or we can also merge #353022 directly, as you prefer.

@dmadisetti
Copy link
Contributor Author

Closed out for the new PR.

@dmadisetti dmadisetti closed this Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: python 8.has: package (new) This PR adds a new package 8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants