Skip to content

Add Bytes parameter - #542

Merged
philippjfr merged 4 commits into
mainfrom
bytes_parameter
Jan 7, 2023
Merged

philippjfr merged 4 commits into
mainfrom
bytes_parameter

Conversation

@philippjfr

Copy link
Copy Markdown
Member

Implements #408

@jbednar jbednar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm glad to have a Parameter supporting bytes, but could this just be a class_ or type_ or bytes=True argument to String to enforce the the bytes type?

Comment thread param/__init__.py Outdated
@MarcSkovMadsen

MarcSkovMadsen commented Oct 7, 2021

Copy link
Copy Markdown
Collaborator

Would I be able to use the Bytes parameter in Panels ReactiveHTML to efficiently transfer files or Arrow dataframes between server and client?

@philippjfr

Copy link
Copy Markdown
Member Author

Would I be able to use the Bytes parameter in Panels ReactiveHTML to efficiently transfer files or Arrow dataframes between server and client?

Could look at that for sure but note that param.DataFrame parameters on ReactiveHTML are already serialized efficiently as a ColumnDataSource.

@jlstevens

Copy link
Copy Markdown
Contributor

Other than Jim's docstring comment, looks good to me!

@jlstevens

Copy link
Copy Markdown
Contributor

I'm glad to have a Parameter supporting bytes, but could this just be a class_ or type_ or bytes=True argument to String to enforce the the bytes type?

Given that a big part of the whole py3 transition was trying to be clear that strings and bytes are different, I think supporting bytes via a type argument to String in this way would be weird. I think a separate Bytes parameter seems like a good idea for semantic clarity.

@jbednar jbednar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good! It's a little odd for the unit test to use an IP address instead of something that would be more naturally stored as bytes, but I guess that's fine, so feel free to merge as-is.

@philippjfr
philippjfr force-pushed the bytes_parameter branch 2 times, most recently from 5d3b91e to 5307ddf Compare January 6, 2023 23:27
@philippjfr
philippjfr merged commit 111cfc1 into main Jan 7, 2023
@philippjfr
philippjfr deleted the bytes_parameter branch January 7, 2023 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants