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

Consider adding special Base.promote_typejoin rules for InlineString #14

Open
bkamins opened this issue Nov 5, 2021 · 1 comment
Open

Comments

@bkamins
Copy link

bkamins commented Nov 5, 2021

Following #4 I would propose that instead of:

julia> Base.promote_typejoin(InlineString1, InlineString3)
InlineString

julia> Base.promote_typejoin(InlineString1, String)
AbstractString

we have:

julia> Base.promote_typejoin(InlineString1, InlineString3)
InlineString3

julia> Base.promote_typejoin(InlineString1, String)
String

I know it technically breaks the contract for Base.promote_typejoin but I think in practice in many cases it is an improvement and I do not see scenarios when it would be a problem (given the intended way strings are used - i.e. that in this case type information should be kind of "ignored" in practice and it is better to do the conversion rather than have container having abstract eltype - unless someone explicitly asks for such a container).

CC @nalimilan @quinnj

@nalimilan
Copy link
Member

Unfortunately the docstring for promote_typejoin requires that it returns a type which is a parent of both input types (and I think map and broadcast rely on this). It's not even clear that it is supposed to be overridden by custom types and that it's technically possible to do that properly. See JuliaLang/julia#38241.

Though it could be useful to have map and broadcast in Base use a kind of promotion function that types could override when appropriate. I imagine they don't use promote_type because in some cases it can lead to loss of precision, but we could have a function which is only implemented when promotion is guaranteed not to lose anything (like here).

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

No branches or pull requests

2 participants