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

Fix: Encode str to bytes before writing to buffered IO #1197

Closed
wants to merge 1 commit into from

Conversation

mesquita
Copy link

@mesquita mesquita commented Jul 9, 2024

If you try something like this:

html_buffer = BytesIO()
report.save_html(html_buffer)

where report is

report = Report(
    metrics=[
        DataDriftPreset(),
    ]
)

after a successful report.run

You'll get:

TypeError: a bytes-like object is required, not 'str'

due to the fact that render in filename.write(render) is a string, not bytes.
In this PR, I'm encoding render to bytes before writing to the buffer.

@mesquita mesquita marked this pull request as ready for review July 9, 2024 18:42
@mesquita mesquita force-pushed the fix/write-to-buffer branch 4 times, most recently from 2ec3c79 to 45c7fe9 Compare July 9, 2024 20:53
@emeli-dral emeli-dral requested a review from mike0sv July 10, 2024 12:29
@mike0sv
Copy link
Collaborator

mike0sv commented Jul 10, 2024

Can't you just use StringIO instead of BytesIO?

@mesquita
Copy link
Author

Can't you just use StringIO instead of BytesIO?

Hi @mike0sv, since save_html accepts str or IO, it should work with BytesIO as well. In my case, it'd be very useful.

I can suggest something like this:

          if isinstance(filename, str):
              with open(filename, "w", encoding="utf-8") as out_file:
                  out_file.write(render)
          elif isinstance(filename, io.BytesIO):
              filename.write(render.encode(encoding="utf-8"))
          elif isinstance(filename, io.StringIO):
              filename.write(render)
          else:
              raise ValueError("filename should be a string or a file-like object")

what do you think?

@mike0sv
Copy link
Collaborator

mike0sv commented Jul 15, 2024

This will not work because of duck typing. Those instance checks will return False for objects created with open(...) for example. Same for typing.BinaryIO and typing.TextIO.
If you need to get html payload, you can use get_html instead of save_html.
As for type annotation, probably we should correct annotation to typing.TextIO or use more flexible check. I don't know how to do this from the top of my head though

@mesquita mesquita closed this Jul 15, 2024
@mesquita mesquita deleted the fix/write-to-buffer branch July 15, 2024 15:04
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.

None yet

2 participants