8000 Add Bytes parameter by philippjfr · Pull Request #542 · holoviz/param · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Bytes parameter #542

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jan 7, 2023
Merged

Add Bytes parameter #542

merged 4 commits into from
Jan 7, 2023

Conversation

philippjfr
Copy link
Member

Implements #408

jbednar
jbednar reviewed Oct 7, 2021
View reviewed changes
8000 Copy link
Member
@jbednar jbednar left a comment

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?

@MarcSkovMadsen
Copy link
Collaborator
MarcSkovMadsen commented Oct 7, 2021 •
edited
Loading

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
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
Contributor

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

@jlstevens
Copy link
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.

@maximlt maximlt mentioned this pull request Nov 11, 2021
Closed
jbednar
jbednar approved these changes Jan 6, 2023
View reviewed changes
Copy link
Member
@jbednar jbednar left a comment

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
@maximlt maximlt mentioned this pull request Feb 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbednar jbednar jbednar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@philippjfr @MarcSkovMadsen @jlstevens @jbednar
0