document x % T #20759

felixrehren · 2017-02-23T14:16:51Z

Cf. #20707

Add docstring

rem(x, T)
mod(x, T)
%(x, T)

For an integer type T, finds z::T such that x ≡ z (mod n), where n is the
cardinality of T and z is restricted to the interval of values representable by T.

julia> 129 % Int8
-127

Could be briefer: strictly speaking, "and z is restricted to ..." is already implied by z::T ?

(Sorry for cruft in commit history. Still trying to figure this out. I guess it will be eliminated by squashing?)

upd

StefanKarpinski · 2017-02-23T20:52:01Z

Pkg test error is unrelated.

tkelman · 2017-02-24T01:00:10Z

base/int.jl

+    %(x, T)
+
+For an integer type `T`, finds `z::T` such that `x` ≡ `z` (mod n), where n is the
+cardinality of `T` and `z` is restricted to the interval of values representable by `T`.


is cardinality of a type a term we expect people to understand without any further elaboration?

Would it be clearer to explain this by saying that x % T returns y :: T such that x ≡ y in the native modular arithmetic of the type T? For example, x % Int8 returns y :: Int8 such that x ≡ y (mod 256) – i.e. one of the representatives from -128 through 127.

Given that both terminologies ("cardinality of a type", "a type's native modular arithmetic") are potentially challenging, perhaps describing this behavior in both ways (to help readers tripped up by one form or the other) and including an example (e.g. that above) would be worthwhile? E.g., "For an integer of type T, finds [... present version in the PR ...]. In other words, finds z::T such that x ≡ z in the native modular arithmetic of type T. For example, x & Int8 returns [...]"?

Fair points. I'm going to try to simplify it --

Sacha0 · 2017-02-24T18:11:59Z

base/int.jl

-For an integer type `T`, finds `z::T` such that `x` ≡ `z` (mod n), where n is the
-cardinality of `T` and `z` is restricted to the interval of values representable by `T`.
+For an integer type `T`, finds `y::T` such that `x` ≡ `y` (mod n), where n is the
+number of elements representable by `T`, and `y` is in `[typemin(T),typemax(T)]`.


Perhaps "number of values representable by T,"? ("elements" seems the right choice when referring to a set, e.g. "number of elements in the set of values of T"; values seems more appropriate coupled to "representable by T".)

Perhaps ", and y is restricted to the values representable by T"? (y not merely being restricted to the interval [typemin(T), typemax(T)].)

Perhaps collapsing those phrases is possible: "where y is restricted to the values representable by T, and n is the number of such values" or so? Best!

Instead of "elements" or "values", I suppose we could just say "integers"?

ararslan · 2017-02-24T18:44:07Z

base/int.jl

+    mod(x, T)
+    %(x, T)
+
+For an integer type `T`, finds `y::T` such that `x` ≡ `y` (mod n), where n is the


I think it should be "find" rather than "finds," since we typically try to stick to the imperative when describing functionality in docstrings.

felixrehren · 2017-02-25T10:10:08Z

It occurred to me that x % T is only defined for x::Integer, in contrast to x % y, and thought this should be part of the documentation.

There's an exception: x % T is also defined for x::Char, but is the same as T(x) in this case. I thought this was not worth documenting.

Sacha0 · 2017-02-25T16:21:01Z

It occurred to me that x % T is only defined for x::Integer, in contrast to x % y, and thought this should be part of the documentation.

There's an exception: x % T is also defined for x::Char, but is the same as T(x) in this case. I thought this was not worth documenting.

Bool-T where T is an integer type and Integer-Type{Bool} methods also exist per methods(rem)?

Sacha0 · 2017-02-25T16:23:32Z

base/int.jl

+    %(x::Integer, T<:Integer)
+
+Find `y::T` such that `x` ≡ `y` (mod n), where n is the number of integers representable
+in `T`, and `y` is an integer in `[typemin(T),typemax(T)]`.


Changed by -> in on purpose. Felt like it sounded more natural. "Represented in" suggests a collection. "Represented by" suggests a mapping to me (like lie algebra element -> matrix operator). I thought Int8 etc. is more like a collection than like a mapping. Is there a convention on this in the docs? Happy to change back

Cheers either way; I lack strong feelings. (by sounds more natural to me, hence the suggestion.) Best!

felixrehren · 2017-02-26T17:00:39Z

@Sacha0 Yes! b % IntX will always map b::Bool to 0 or 1, same as convert. On the other hand, x % Bool maps x to whether x is odd. I can imagine they would go away if Bool stops being a number, for example. Should these be additionally documented?

StefanKarpinski · 2017-02-26T18:16:26Z

We should consider whether the x % Char methods should be deprecated. I think we should probably just have conversion to Char instead.

Sacha0 · 2017-02-26T18:25:55Z

b % IntX will always map b::Bool to 0 or 1, same as convert. On the other hand, x % Bool maps x to whether x is odd.

We should consider whether the x % Char methods should be deprecated. I think we should probably just have conversion to Char instead.

Perhaps the Bool methods should go away as well? isodd(x) seems much clearer than x % Bool. Best!

ararslan · 2017-02-26T19:44:20Z

base/int.jl

 end

+"""
+    rem(x::Integer, T<:Integer)


I think this notation is nonstandard with regards to the rest of the Base docstrings. Also note that <:Integer isn't entirely accurate, since you can't do something like 1 % BigInt.

StefanKarpinski · 2017-02-26T23:05:20Z

I feel like as long as Bool is an integer type having x % Bool makes sense, but if we make it a non-integer type then we should probably remove those methods.

rfourquet · 2017-03-28T04:39:35Z

base/int.jl

+    %(x::Integer, T<:Integer)
+
+Find `y::T` such that `x` ≡ `y` (mod n), where n is the number of integers representable
+in `T`, and `y` is an integer in `[typemin(T),typemax(T)]`.


With this definition would it make sense to define (extend) this operation for T == BigInt ? (but somehow, till now I understood x % T as a cheap bit-level cast operation, which would not work for BigInt...)

Sure, it would make sense semantically. Since nothing is a cheap bit-level operation on BigInts, there's no harm. Since the range of BigInts is unbounded, n % BigInt is equivalent to a convert. In the other direction, b % Int can be done by doing a single word load.

tkelman · 2017-03-28T12:39:41Z

@ararslan's comment was not addressed

StefanKarpinski · 2017-03-28T13:02:24Z

We have a new policy of merging documentation PRs quickly and then iterating on them. I don't even see what comment you are talking about, so if it still needs fixing, if someone who sees the problem could take care of it, that would be lovely. Meanwhile, this produces this warning during a build:

WARNING: replacing docs for 'Base.rem :: Union{}' in module 'Base'.

tkelman · 2017-03-28T13:18:58Z

Thought that was a temporary experiment for Docathon.

Scroll up.

I think this notation is nonstandard with regards to the rest of the Base docstrings.

felixrehren added 4 commits February 14, 2017 08:31

Merge pull request #2 from JuliaLang/master

2d17dd9

upd

Merge branch 'master' of https://github.com/felixrehren/julia

c580272

document x % T

39e93ad

codeify code

f42e294

kshyatt added the docs This change adds or pertains to documentation label Feb 23, 2017

StefanKarpinski approved these changes Feb 23, 2017

View reviewed changes

Sacha0 approved these changes Feb 23, 2017

View reviewed changes

tkelman reviewed Feb 24, 2017

View reviewed changes

copyedit [ci skip]

17f5c65

Sacha0 reviewed Feb 24, 2017

View reviewed changes

ararslan reviewed Feb 24, 2017

View reviewed changes

felixrehren added 2 commits February 25, 2017 10:01

element -> integer [ci skip]

005f2a7

add type signature [ci skip]

311e0bf

Sacha0 reviewed Feb 25, 2017

View reviewed changes

ararslan reviewed Feb 26, 2017

View reviewed changes

rfourquet reviewed Mar 28, 2017

View reviewed changes

StefanKarpinski approved these changes Mar 28, 2017

View reviewed changes

StefanKarpinski merged commit 407995a into JuliaLang:master Mar 28, 2017

tkelman mentioned this pull request Mar 28, 2017

WARNING: replacing docs for 'foo :: Union{}' in module 'Main'. (0.6 only, new parameterization syntax) #21190

Closed

ararslan mentioned this pull request Mar 28, 2017

Correct notation used in the rem/mod docstring #21198

Merged

iamnapo mentioned this pull request Apr 5, 2017

[MISTAKE] #21282

Closed

ararslan mentioned this pull request May 9, 2017

document rem(x,T) / x % T for T a type #20707

Closed

felixrehren deleted the fr/docrem branch May 15, 2017 08:19

Uh oh!

document x % T #20759

document x % T #20759

Uh oh!

Conversation

felixrehren commented Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StefanKarpinski commented Feb 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sacha0 Feb 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixrehren Feb 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixrehren commented Feb 25, 2017

Uh oh!

Sacha0 commented Feb 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixrehren Feb 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixrehren commented Feb 26, 2017

Uh oh!

StefanKarpinski commented Feb 26, 2017

Uh oh!

Sacha0 commented Feb 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanKarpinski commented Feb 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tkelman commented Mar 28, 2017

Uh oh!

StefanKarpinski commented Mar 28, 2017

Uh oh!

tkelman commented Mar 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

felixrehren commented Feb 23, 2017 •

edited

Loading

Sacha0 Feb 24, 2017 •

edited

Loading

felixrehren Feb 25, 2017 •

edited

Loading

Sacha0 commented Feb 25, 2017 •

edited

Loading

felixrehren Feb 26, 2017 •

edited

Loading