Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

FEAT: Adding pickle support to quaddtype#232

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
SwayamInSync merged 9 commits intonumpy:mainfromSwayamInSync:hash
Nov 26, 2025

Conversation

@SwayamInSync
Copy link
Member

This PR adds the pickle support as per#231
Usage:

original=np.array([1.5,2.5,3.5,4.5],dtype=QuadPrecDType(backend=backend))# Pickle and unpicklepickled=pickle.dumps(original)unpickled=pickle.loads(pickled)

Usingnp.savez

arr1=np.array([1.5,2.5,3.5],dtype=QuadPrecDType(backend=backend))arr2=np.array([[1.0,2.0], [3.0,4.0]],dtype=QuadPrecDType(backend=backend))np.savez(fname,array1=arr1,array2=arr2)loaded=np.load(fname,allow_pickle=True)# allow_pickle must be True during loadingloaded_arr1=loaded['array1']loaded_arr2=loaded['array2']

@SwayamInSync
Copy link
MemberAuthor

We might not need to useNPY_LIST_PICKLE as when NumPy pickles a QuadPrecDType array:

  • It saves the dtype (using ourreduce) (QuadPrecDType, ('sleef',))
  • It saves the shape
  • It saves the data,PyArray_ToString returns raw 128-bit bytes

StringDType needsNPY_LIST_PICKLE because:

  • Its memory contains pointers to Python strings
  • PyArray_ToString would just copy worthless pointer values
  • So it uses_getlist_pkl to extract actual string objects

OurQuadPrecDType stores actual numeric values in memory, soPyArray_ToString works perfectly

@SwayamInSync
Copy link
MemberAuthor

hash implementation might require some careful handlinghttps://github.com/python/cpython/blob/20b69aac0d19a5e5358362410d9710887762f0e7/Python/pyhash.c#L87

I'll do this in a different PR and then we can close the#231

Copy link
Member

@jorenhamjorenham left a comment

Choose a reason for hiding this comment

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

stub changes looks good to me

@juntyr
Copy link
Contributor

Why is the backend provided as an integer? A string literal might be a lot clearer, or if it has to be an int maybe we could add an int enum?

@SwayamInSync
Copy link
MemberAuthor

Why is the backend provided as an integer? A string literal might be a lot clearer, or if it has to be an int maybe we could add an int enum?

Internallybackendis defined as enum, we provide the friendly API for the user to add input as string. For the sake of static tools, IDE suggestions I think defining as enum might make sense maybe@jorenham can give some inputs here

@jorenham
Copy link
Member

I suppose you could doBackend: typing.TypeAlias = typing.Literal[1, 2], or maybe even anenum.IntEnum

Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
@SwayamInSync
Copy link
MemberAuthor

@ngoldbaum a gentle reminder here for review

@ngoldbaum
Copy link
Member

@SwayamInSync I'd appreciate it if you could give me more than a couple of days - especially over a weekend - before pinging me on PR reviews. I had your PRs on my to-do list.

Copy link
Member

@ngoldbaumngoldbaum left a comment

Choose a reason for hiding this comment

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

The C code and tests overall look good. I didn't review the tests in detail but I'm glad to see such extensive coverage of usages.

assertunpickled.flags.f_contiguous==original.flags.f_contiguous

@pytest.mark.parametrize("backend", ["sleef","longdouble"])
deftest_pickle_reduce_method(self,backend):
Copy link
Member

Choose a reason for hiding this comment

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

My only comment is that this test is a little too tightly coupled to the implementation to be useful.

You could, for example, decide to use__getnewargs__ instead of__reduce__ but then you'd have to update this test too.https://docs.python.org/3/library/pickle.html#object.__getnewargs__.

Copy link
MemberAuthor

@SwayamInSyncSwayamInSyncNov 25, 2025
edited
Loading

Choose a reason for hiding this comment

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

Right, I think I should just remove this test? as we already have tests which are just focussed on working of the pickle rather than implementation?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm suggesting to delete the test.

@SwayamInSync
Copy link
MemberAuthor

@SwayamInSync I'd appreciate it if you could give me more than a couple of days - especially over a weekend - before pinging me on PR reviews. I had your PRs on my to-do list.

So sorry, I was working this weekend as well so slightly forgot the track of days :)

@SwayamInSync
Copy link
MemberAuthor

Merging this in

@SwayamInSyncSwayamInSync merged commitf2a4bec intonumpy:mainNov 26, 2025
11 checks passed
@SwayamInSyncSwayamInSync added this to thev1.0 milestoneDec 8, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@ngoldbaumngoldbaumngoldbaum approved these changes

@jorenhamjorenhamjorenham approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

v1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@SwayamInSync@juntyr@jorenham@ngoldbaum

[8]ページ先頭

©2009-2025 Movatter.jp