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

ByteSize.human_readable with an optional separator (e.g. whitespace) between value and unit #8668

Closed
3 of 13 tasks
jks15satoshi opened this issue Jan 29, 2024 · 2 comments · Fixed by #8706
Closed
3 of 13 tasks
Assignees

Comments

@jks15satoshi
Copy link
Contributor

jks15satoshi commented Jan 29, 2024

Initial Checks

  • I have searched Google & GitHub for similar requests and couldn't find anything
  • I have read and followed the docs and still think this feature is missing

Description

Currently, the return value of ByteSize.human_readable does not have any separator between the value and unit.

>>> size = ByteSize(12345678)
>>> size.human_readable()
'11.8MiB'

It looks a little bit weird to me, and I think it could be better if there's a space separating the value and unit. Though there are many ways to handle this need, it may be a better idea to provide an optional argument with this method? For example:

ByteSize.human_readable(decimal: bool = False, separator: bool = False) -> str

When I switch the separator on:

>>> size = ByteSize(12345678)
>>> size.human_readable(separator=True)
'11.8 MiB'

It's probably easy to be implemented, so I can create a PR to implement this feature if it will be acceptable.

What do you think about it?

Affected Components

@sydney-runkle
Copy link
Member

@jks15satoshi,

Go for it! Just makes sure that by default, the separator is '' so that this isn't a breaking change. Thanks for the suggestion. Ping me when you need a review :).

@jks15satoshi
Copy link
Contributor Author

@sydney-runkle,

Thanks. PR has been created :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants