Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metadata passthrough and docstring to glue function. #34

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@mpacer
Copy link
Member

commented Mar 21, 2019

This does not address the fact that it seems to break the tests by making it so it always appears to be displaying in the final result.

I was having a little bit of difficulty thinking through the test, they are not super intuitive. Could I get some guidance around that?

Add metadata passthrough and docstring to glue function.
This does not address the fact that it seems to break the tests by
making it so it always appears to be displaying in the final result.

@mpacer mpacer requested a review from MSeal Mar 21, 2019

@MSeal
Copy link
Member

left a comment

For testing I'd expand the arguments to include input_metadata and add a case to test_glue and test_glue_display_only which proves that the user input makes it into the mock_display call. This proves that the logic is applied against ipython's interface.

@@ -64,8 +66,18 @@ def glue(name, scrap, encoder=None, display=None):
The data protocol name to respect in persisting data
display: any (optional)
An indicator for ...
metadata: dict or None(optional )

This comment has been minimized.

Copy link
@MSeal

MSeal Mar 21, 2019

Member

whitespacing

@@ -85,11 +97,13 @@ def glue(name, scrap, encoder=None, display=None):

# Only store data that can be stored (purely display scraps can skip)
if encoder != "display":
data, metadata = _prepare_ipy_data_format(

This comment has been minimized.

Copy link
@MSeal

MSeal Mar 21, 2019

Member

why the extra newline?

data, prepared_metadata = _prepare_ipy_display_format(
name, raw_data, raw_metadata
)
metadata.update(prepared_metadata)

This comment has been minimized.

Copy link
@MSeal

MSeal Mar 21, 2019

Member

Nice

@@ -102,7 +116,10 @@ def glue(name, scrap, encoder=None, display=None):
raw_data, raw_metadata = IPython.core.formatters.format_display_data(
scrap, **display_kwargs
)
data, metadata = _prepare_ipy_display_format(name, raw_data, raw_metadata)
data, prepared_metadata = _prepare_ipy_display_format(

This comment has been minimized.

Copy link
@MSeal

MSeal Mar 21, 2019

Member

I think black was converting this back to the original when I ran it ¯_(ツ)_/¯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.