Skip to content

Conversation

@cfs-pure
Copy link
Contributor

@cfs-pure cfs-pure commented Nov 1, 2023

Replaced the use of old-style formatting strings in str methods.

At first, that seems like a dated but harmless practice, but unfortunately

While dated and seemingly a harmless practice, OSlash actually uses it in an incorrect way.

If a program using OSlash deals with types other than strings, then the str methods for classes like Left, Right, and Either, OSlash directly writes:

return '%s' % value

where value is an arbitrary type.

Unfortunately, '%s' % value is only guaranteed to work if the value is a string, since Python will not call str() on it.

(Well, more precisely, '%s' % value is guaranteed to fail if the value happens to be a tuple of a certain kind, since Python will try to unpack the tuple into the string formatting operation)

The upshot is that such programs will get TypeErrors. Instead, use Python 3.8+ f-strings to guarantee that the value is converted to a string.

cfs-pure and others added 2 commits November 1, 2023 16:51
Replaced the use of old-style formatting strings in __str__ methods.

At first, that seems like a dated but harmless practice, but unfortunately

While dated and seemingly a harmless practice, OSlash actually uses it
in an incorrect way.

If a program using OSlash deals with types other than strings, then the
__str__ methods for classes like `Left`, `Right`, and `Either`, OSlash
directly writes:

    return '%s' % value

where `value` is an arbitrary type.

Unfortunately, '%s' % value is only guaranteed to work if the value
is a string, since Python will not call `str()` on it.

(Well, more precisely, `'%s' % value` is guaranteed to *fail* if the
value happens to be a tuple of a certain kind, since Python will try
to unpack the tuple into the string formatting operation)

The upshot is that such programs will get TypeErrors.  Instead, use
Python 3.8+ f-strings to guarantee that the value is converted to a
string.
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@dbrattli dbrattli merged commit 43d3b10 into dbrattli:master Nov 16, 2025
11 checks passed
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.

3 participants