Skip to content
This repository has been archived by the owner on Jan 13, 2021. It is now read-only.

Added Huffman decoder and tests #5

Merged
merged 3 commits into from
Jan 31, 2014
Merged

Conversation

Sriram137
Copy link
Contributor

Kinda new to python. Could you review it, so I can fix any code quality issues which might be present.

from hyper.http20.huffman_constants import REQUEST_CODES,REQUEST_CODES_LENGTH,RESPONSE_CODES,RESPONSE_CODES_LENGTHS

def _pad_binary(bin_str,req_len = 8):
return max(0,(req_len) - len(bin_str)) * '0' + bin_str
Copy link
Member

Choose a reason for hiding this comment

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

Can you leave a space after the comma between method arguments? E.g max(a, b).

Copy link
Member

Choose a reason for hiding this comment

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

This applies to function definitions, e.g. def _pad_binary(bin_str, req_len=8). Also, in function definitions where some keywords have default values, don't put a space around the = sign. Do it like I just did in the example.

@Lukasa
Copy link
Member

Lukasa commented Jan 29, 2014

This looks really excellent, thankyou! I've left some comments inline, but they're all stylistic. Let me know when you're done and I'll swing through it again. =)

26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26,
26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26,
26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26,
26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26]
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this closing square bracket down to the next line?

@Lukasa
Copy link
Member

Lukasa commented Jan 30, 2014

Ok, two small further markups, pretty trivial. =)

@Lukasa Lukasa merged commit f86adec into python-hyper:master Jan 31, 2014
@Lukasa
Copy link
Member

Lukasa commented Jan 31, 2014

Thanks so much!

I've merged it manually, since there were a few small changes I wanted to make. I've added you to the Contributors list. Again, thankyou, this is work I desperately needed done and couldn't find the time to do! 🍰 🍪 ⭐ 🌟

@Lukasa Lukasa mentioned this pull request Jan 31, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants