Skip to content

Conversation

@PallHaraldsson
Copy link
Contributor

@PallHaraldsson PallHaraldsson commented Oct 14, 2025

It is public, already used in code, and help?> uabs doesn't find ii, only:

help?> Base.uabs
│ Warning

│ The following bindings may be internal; they may change or be removed in future versions:

│ • Base.uabs
..

and since it is public it will likely never be removed (or be changed, is quite simple); and I'm not sure what triggers it but that Warning should go. Please add to this PR to make it disappear before merging it.

https://github.com/search?q=repo%3AJuliaDSP%2FDSP.jl%20uabs&type=code

[I doubt it should be exported, is quite short so might conflict... would have been ideal from the start.]

@adienes
Copy link
Member

adienes commented Oct 14, 2025

I don't think the (negative) parenthetical is needed as the sentence immediately goes on to describe exactly when overflow occurs. and rather than putting "this never occurs with uabs" in the docstring itself, I'd just add it to the see also list.

@PallHaraldsson

This comment was marked as resolved.

@adienes
Copy link
Member

adienes commented Oct 14, 2025

there are a lot of questions in there and I'm not sure which you intend for me to answer. as a side note --- and maybe this is just a me problem --- I find it quite difficult to follow what you're saying when you write in so many sentence fragments and run-ons.

as for a specific diff, I would do something like

See also: [`abs2`](@ref), [`unsigned`](@ref), [`sign`](@ref), [`Base.uabs.`](@ref)

and revert the other parts of this PR.

@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Oct 14, 2025

One more thing, unlike abs, uabs doesn't, but maybe should too work for all numbers:
julia> Base.uabs(-1.0)
ERROR: MethodError: no method matching uabs(::Float64)

Return the absolute value of x, possibly returning a different type should the operation be susceptible to overflow.

I.e. it's unclear it needs to change to Unsigned. Could work for any real, just then do an abs, Float64 -> Float64. [And if I go with your simplest change] do you know how also to get rid of the Warning, where it comes from?

a lot of questions in there .. maybe this is just a me problem

Not a you problem, I'm sure it's a me problem, my ADHD, also affecting more than you... sorry. I wrote very quickly, maybe I can trim. Since you were commenting on "(negative) parenthetical" then yes I was answering/asking you. Maybe I'm overthinking uabs... :) At first I was simply checking if it were public and for sure should be used (more), likely only for smaller integer types, not must users need to worry about.

@adienes
Copy link
Member

adienes commented Oct 14, 2025

I suppose uabs(::Float64) technically could be defined but I don't think it's necessary. uabs is really meant for integers. the warning you're seeing is because uabs is not marked public until 1.13

for sure should be used (more)

it's not the documentation's (primary) job to instruct users how much or how little to use certain tools. rather, its job is to inform users of the tools that exist and how they work.

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