Page MenuHomePhabricator

docket: make compatible with py3.6, where Struct.format is bytes
AbandonedPublic

Authored by martinvonz on May 25 2021, 7:55 PM.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.May 25 2021, 7:55 PM
marmoute requested changes to this revision.May 28 2021, 9:10 AM
marmoute added a subscriber: marmoute.

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

This revision now requires changes to proceed.May 28 2021, 9:10 AM

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

I'm not sure I understand. Can you show me how?

$ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'bytes'>
$ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'bytes'>
$ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'str'>
$ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'str'>
marmoute added a comment.EditedMay 28 2021, 9:32 AM

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

I'm not sure I understand. Can you show me how?

$ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'bytes'>
$ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'bytes'>
$ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'str'>
$ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'str'>

Okay, so my assertion isn't correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?

martinvonz abandoned this revision.May 28 2021, 9:34 AM

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

I'm not sure I understand. Can you show me how?

$ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'bytes'>
$ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'bytes'>
$ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'str'>
$ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'str'>

Okay, so my assertion is correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

I'm not sure I understand. Can you show me how?

$ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'bytes'>
$ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'bytes'>
$ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'str'>
$ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'str'>

Okay, so my assertion is correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?

I see you are abandoning this revision, can you share details about your new plan ?

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

I'm not sure I understand. Can you show me how?

$ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'bytes'>
$ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'bytes'>
$ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'str'>
$ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'str'>

Okay, so my assertion is correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?

AFAICT your assertion was wrong: the struct module just isn't preserving the input type at all, and always emits str on 3.7+, but does bytes on 3.6. So we're not mixing bytes/str input types to struct, but instead struct on 3.6 is ninja-converting back to bytes "for" us.

A pycompat wrapper would make sense for this if we see more than one case of this. Is that likely?

I see you are abandoning this revision, can you share details about your new plan ?

I believe he found the interaction too frustrating and is dealing with other things.

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

I'm not sure I understand. Can you show me how?

$ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'bytes'>
$ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'bytes'>
$ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'str'>
$ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'str'>

Okay, so my assertion is correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?

AFAICT your assertion was wrong: the struct module just isn't preserving the input type at all, and always emits str on 3.7+, but does bytes on 3.6. So we're not mixing bytes/str input types to struct, but instead struct on 3.6 is ninja-converting back to bytes "for" us.

Yep, this is a typo on my side (typing is hard) (is right because isn't right)

A pycompat wrapper would make sense for this if we see more than one case of this. Is that likely?

It feels wrong to me to have this kind of compatibility weirdness crawl into normal code. I though more about it and in this case I think the simplest would probably to have the format for INDEX_HEADER in a INDEX_HEADER _FMT constant and concatenate with that.

Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.

I'm not sure I understand. Can you show me how?

$ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'bytes'>
$ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'bytes'>
$ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
<class 'str'>
$ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
<class 'str'>

Okay, so my assertion is correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?

AFAICT your assertion was wrong: the struct module just isn't preserving the input type at all, and always emits str on 3.7+, but does bytes on 3.6. So we're not mixing bytes/str input types to struct, but instead struct on 3.6 is ninja-converting back to bytes "for" us.

Yep, this is a typo on my side (typing is hard) (is right because isn't right)

A pycompat wrapper would make sense for this if we see more than one case of this. Is that likely?

It feels wrong to me to have this kind of compatibility weirdness crawl into normal code. I though more about it and in this case I think the simplest would probably to have the format for INDEX_HEADER in a INDEX_HEADER _FMT constant and concatenate with that.

Yeah, it feels a bit wrong to me too, but I don't see any great options, esp. w/ the pycompat string tool already doing what we need.

Your solution feels fine-ish, with the caveat that it's more prone to falling out of sync (though I suppose tests would catch that very quickly). I queued this, but if you like the two-constant approach send that as a follow-up?

I will follow the guide here and make docket compatible with py3.6. This is what I want, and I can focus on my australian writings work once this is done. That will make me happy!

Thanks for sharing this article.

Awesome and interesting article. Great things you've always shared with us. Thanks. Just continue composing this kind of post. automatic reply text message

We accordingly, pride ourselves as one of the most committed south africa music site, propelled with the sole motivation behind handing out new and selective Amapiano, Gqom and Afro house songs.
De mthuda mp3 download

ykglobal added a subscriber: ykglobal.EditedThu, May 12, 9:45 AM

Our goals are to assist our client’s business, to resolve complex transactions on behalf and to build confidence with up to date financial information. Bookkeeping Services

H1 Economics is the short form of “Higher 1” Economics. Students have a choice between H1 and H2 Economics. However, one of the biggest misconceptions is that you need to study less for H1 economics. Both H1 and H2 Economics have similar topics, skills and assessment objectives to cover. In H1 Economics, however, students only have to attempt Paper 1, consisting of two compulsory case-study questions. Economics Tuition

Dr. Chiro Pte. Ltd. is a professionally trained chiropractic and holistic health care practice committed to revitalising and transforming your mental and physical health through non-surgical and non-drug procedures. Top Chiropractor Singapore

Irwin’s Study is an unique tuition centre that specialises in General Paper (GP) Tuition. Founded in 2009, Irwin’s Study has taught, inspired & impacted hundreds of students who have walked through our doors. GP Tuition