#31591 closed enhancement (fixed)

Expose MemoryAllocator as python package

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.4
Component: cython Keywords:
Cc: Vincent Delecroix, Matthias Köppe, Erik Bray, Travis Scrimshaw Merged in:
Authors: Jonathan Kliem Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 1d84a20 (Commits, GitHub, GitLab) Commit: 1d84a20f691f55ea79a66793852add6709a417d7
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

We expose MemoryAllocator as an independent Python package.

Indeed, it doesn't depend on anything Sage-specific, and it can be useful for other Cython projects.

In order to remove the dependency on cysignals, which at the moment doesn't work on Windows without Cygwin, we remove sig_block/sig_unblock from the allocation. This seems not harmful as the allocation via MemoryAllocator should not be called within sig_on/sig_off anyway.

Change History (35)

comment:1 Changed 20 months ago by gh-kliem

Milestone: sage-9.3sage-9.4

comment:2 Changed 20 months ago by Samuel Lelièvre

Description: modified (diff)

comment:3 Changed 20 months ago by gh-kliem

Branch: u/gh-kliem/outsource_memory_allocator
Commit: b2c75d84bb0de856bf0d6e586e64b20f3466ec23
Description: modified (diff)

New commits:

cd6212dexpose memoryallocator as extra package
b2c75d8replace by memory_allocator package

comment:4 Changed 20 months ago by gh-kliem

Description: modified (diff)

Sorry, back to modified description.

comment:5 Changed 20 months ago by gh-kliem

Cc: Vincent Delecroix Matthias Köppe Erik Bray added
Status: newneeds_review

comment:6 Changed 20 months ago by Matthias Köppe

The python package will need a pyproject.toml, in which you declare the build dependency on Cython

comment:7 in reply to:  6 Changed 20 months ago by gh-kliem

Replying to mkoeppe:

The python package will need a pyproject.toml, in which you declare the build dependency on Cython

https://github.com/kliem/memory_allocator/blob/main/pyproject.toml:

[build-system]
requires = ["setuptools", "wheel", "Cython"]
build-backend = "setuptools.build_meta"

I more or less copied it from pplpy.

comment:8 Changed 18 months ago by gh-kliem

Cc: Travis Scrimshaw added

comment:9 Changed 18 months ago by Travis Scrimshaw

Your SPKG file needs a bit more information. Also, even though this already is a part of Sage, we should check on sage-devel to confirm there are no objections to short-circuiting adding a new standard package.

comment:10 Changed 18 months ago by Dima Pasechnik

you have an invitation to Sagemath github "team"; once a member, you'd be able to transfer ownership to sagemath

I also noted in an issue on your repo that one should import there the full history of the code taken out of sagemath.

comment:11 Changed 18 months ago by git

Commit: b2c75d84bb0de856bf0d6e586e64b20f3466ec234589081fc87f50de1dddc61b06c3838abe327988

Branch pushed to git repo; I updated commit sha1. New commits:

4d8927eMerge branch 'u/gh-kliem/outsource_memory_allocator' of git://trac.sagemath.org/sage into u/gh-kliem/outsource_memory_allocator2
4589081updated to newest beta

comment:12 in reply to:  10 Changed 18 months ago by gh-kliem

Replying to dimpase:

you have an invitation to Sagemath github "team"; once a member, you'd be able to transfer ownership to sagemath

I also noted in an issue on your repo that one should import there the full history of the code taken out of sagemath.

Thank you Dima for setting this up. I also imported the history of memory_allocator/memory.pxd, which was just stolen from cysignals.

comment:13 in reply to:  10 Changed 18 months ago by gh-kliem

Replying to dimpase:

you have an invitation to Sagemath github "team"; once a member, you'd be able to transfer ownership to sagemath

I also noted in an issue on your repo that one should import there the full history of the code taken out of sagemath.

I added dependencies and updated the links. What else is missing?

Note that the project is not on PyPi? yet. I'm waiting for an ok of someone, that the prerelease is somewhat acceptable.

comment:14 Changed 18 months ago by gh-kliem

Work issues: move repository to github/sagemath, make a release on PyPimake a release on PyPi

comment:15 Changed 18 months ago by git

Commit: 4589081fc87f50de1dddc61b06c3838abe32798844a0df452863633bf415a96bc406d27b81738f5e

Branch pushed to git repo; I updated commit sha1. New commits:

fd87c9531866: fixing 3 doctests
44a0df4Merge branch 'u/slabbe/31866' of git://trac.sagemath.org/sage into u/gh-kliem/outsource_memory_allocator

comment:16 Changed 18 months ago by git

Commit: 44a0df452863633bf415a96bc406d27b81738f5eb3696a62e7a39a67418a7d24538394d6d8b51713

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b3696a6Merge branch 'develop' of git://trac.sagemath.org/sage into u/gh-kliem/outsource_memory_allocator

comment:17 Changed 18 months ago by git

Commit: b3696a62e7a39a67418a7d24538394d6d8b51713402e8e4d2941c9af52d56e3012d57c6f3d74acba

Branch pushed to git repo; I updated commit sha1. New commits:

402e8e4update to newest beta

comment:18 Changed 18 months ago by gh-kliem

Sorry about the mess...

comment:19 Changed 18 months ago by Dima Pasechnik

there is something silly going on with the versions, I guess, I get

$ less logs/pkgs/memory_allocator-0.1.0-beta.1.log
Error: Installing old-style SPKGs is no longer supported

comment:20 Changed 18 months ago by Dima Pasechnik

it is due to missing tarball=... line in checksums.ini

comment:21 Changed 18 months ago by Dima Pasechnik

Status: needs_reviewneeds_work

OK, it installis after

  • build/pkgs/memory_allocator/checksums.ini

    diff --git a/build/pkgs/memory_allocator/checksums.ini b/build/pkgs/memory_allocator/checksums.ini
    index 14e6cf2a2b..fb9511de83 100644
    a b  
     1tarball=memory_allocator--VERSION.tar.gz
    12sha1=d0eeb3be4462a41b316086a33f4dfbca08b887b4
    23md5=74c8a21dd6e79452bae99881d40ab2f9
    34cksum=3681970071
  • build/pkgs/memory_allocator/spkg-install.in

    diff --git a/build/pkgs/memory_allocator/spkg-install.in b/build/pkgs/memory_allocator/spkg-install.in
    index deba1bb42b..058b1344dc 100644
    a b  
    1 cd src && sdh_pip_install .
     1cd src
     2
     3sdh_pip_install .

I have also added

$ cat build/pkgs/memory_allocator/install-requires.txt
memory_allocator

comment:22 Changed 18 months ago by git

Commit: 402e8e4d2941c9af52d56e3012d57c6f3d74acbadde45e2f063a163af0db11757944de1d63dad8b7

Branch pushed to git repo; I updated commit sha1. New commits:

dde45e2small fixes in build files

comment:23 Changed 18 months ago by gh-kliem

Status: needs_workneeds_review

comment:24 Changed 18 months ago by Matthias Köppe

Let's please use a pypi URL in upstream_url. For example, use ./sage -package create memory_allocator --pypi --type standard

comment:25 Changed 18 months ago by git

Commit: dde45e2f063a163af0db11757944de1d63dad8b7db8330c24af995f9497f0931f5555e3892340676

Branch pushed to git repo; I updated commit sha1. New commits:

db8330cmake a release on pypi

comment:26 Changed 18 months ago by gh-kliem

Work issues: make a release on PyPi

comment:27 in reply to:  24 Changed 18 months ago by gh-kliem

Replying to mkoeppe:

Let's please use a pypi URL in upstream_url. For example, use ./sage -package create memory_allocator --pypi --type standard

That was the plan all along. I was just unsure of whether this was ready to push on pypi. But then again, one can always to a version 0.1.0, if needed.

comment:28 Changed 18 months ago by Matthias Köppe

The upstream url in the format that the other packages use is better because it will make upgrades easier

comment:29 Changed 18 months ago by Dima Pasechnik

Reviewers: Dima Pasechnik
Status: needs_reviewpositive_review

otherwise, alles gut

comment:30 Changed 18 months ago by Samuel Lelièvre

Might make sense to advertise this new package on the cython-users mailing list.

comment:31 Changed 18 months ago by git

Commit: db8330c24af995f9497f0931f5555e38923406761d84a20f691f55ea79a66793852add6709a417d7
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

1d84a20update upstream url to be more generic

comment:32 Changed 18 months ago by gh-kliem

Status: needs_reviewpositive_review

comment:33 in reply to:  29 ; Changed 18 months ago by gh-kliem

Replying to dimpase:

otherwise, alles gut

Thank you.

I updated the url and wrote a brief advertisment on the cython-users mailing list.

comment:34 in reply to:  33 Changed 18 months ago by Samuel Lelièvre

Replying to gh-kliem:

I updated the url and wrote a brief advertisement on the cython-users mailing list.

For reference the post on cython-users is at:

comment:35 Changed 18 months ago by Volker Braun

Branch: u/gh-kliem/outsource_memory_allocator1d84a20f691f55ea79a66793852add6709a417d7
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.