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

Deprecate prime number functions (isprime, primes, primesmask, factor) #16481

Merged
merged 3 commits into from
May 27, 2016

Conversation

simonbyrne
Copy link
Contributor

@simonbyrne simonbyrne commented May 20, 2016

See #16357.

Current functionality has been moved to https://github.com/JuliaMath/Primes.jl

@tkelman
Copy link
Contributor

tkelman commented May 20, 2016

Cool. This should wait until Primes.jl is registered before merging.

edit: and move the docs into the readme, and add a NEWS.md mention

@tkelman
Copy link
Contributor

tkelman commented May 20, 2016

Remaining mentions:

contrib/BBEditTextWrangler-julia.plist:645:            <string>isprime</string>
doc/manual/arrays.rst:322:    julia> x[map(isprime, x)]
doc/stdlib/numbers.rst:458:.. function:: isprime(x::Integer) -> Bool
doc/stdlib/numbers.rst:466:       julia> isprime(3)
doc/stdlib/numbers.rst:469:.. function:: isprime(x::BigInt, [reps = 25]) -> Bool
doc/stdlib/numbers.rst:477:       julia> isprime(big(3))
contrib/BBEditTextWrangler-julia.plist:861:            <string>primes</string>
contrib/BBEditTextWrangler-julia.plist:862:            <string>primesmask</string>
doc/stdlib/numbers.rst:480:.. function:: primes([lo,] hi)
doc/stdlib/numbers.rst:486:.. function:: primesmask([lo,] hi)
doc/stdlib/numbers.rst:490:   Returns a prime sieve, as a ``BitArray``\ , of the positive integers (from ``lo``\ , if specified) up to ``hi``\ . Useful when working with either primes or composite numbers.
contrib/BBEditTextWrangler-julia.plist:435:            <string>factor</string>
doc/stdlib/math.rst:1362:.. function:: factor(n) -> Dict
doc/stdlib/math.rst:1366:   Compute the prime factorization of an integer ``n``\ . Returns
 a dictionary. The keys of the dictionary correspond to the factors, and hence are of the same type as ``n``\ . The value associated with each key indicates the number of times the factor appears in the factorization.
doc/stdlib/math.rst:1370:       julia> factor(100) # == 2*2*5*5

edit: oh, and base/sysimg.jl, of course

@simonbyrne
Copy link
Contributor Author

The question is how best to handle deprecations of functions: should we do the same thing as #13897, where we throw an error (instead of depwarn)?

@tkelman
Copy link
Contributor

tkelman commented May 22, 2016

I actually liked the way you had it where the old functions were just moved to being included from deprecated.jl. No errors that way, just warnings.

@simonbyrne
Copy link
Contributor Author

simonbyrne commented May 22, 2016

Yeah, I wasn't sure what was the best way (I just copied what Combinatorics.jl did), which is why I kept it as a separate commit. We should figure out a consistent way to do this, especially as we're going to move more stuff out of Base.

The trouble is figuring out a way so that doing the correct thing, e.g. something like using Primes; isprime(7), doesn't throw a warning or error for deprecation, duplicate export or method overwriting.

@tkelman
Copy link
Contributor

tkelman commented May 24, 2016

So which (if any?) of these commits is able to do that?

@simonbyrne
Copy link
Contributor Author

Once we merge this, my plan is to add something like the following to Primes.jl:

if VERSION >= v"0.6-"
    export isprime, primes, primesmask, factor
elseif VERSION >= v"0.5-dev+XXXX"
    import Base: isprime, primes, primesmask, factor
end

Then using Primes shouldn't cause a problem on any version (after 0.4).

@tkelman
Copy link
Contributor

tkelman commented May 24, 2016

using isdefined would probably be more accurate in capturing exactly when the deprecations get removed.

@simonbyrne
Copy link
Contributor Author

Okay, so we could use:

if VERSION >= v"0.5-dev+XXXX"
    if isdefined(Base,:isprime)
        import Base: isprime, primes, primesmask, factor
    else
        export isprime, primes, primesmask, factor
    end
end

Either way, we still need the commit no for this change.

@tkelman
Copy link
Contributor

tkelman commented May 25, 2016

Yep. So if we prefer the error-causing version, merge this as is?

@simonbyrne
Copy link
Contributor Author

Yes, I think it should be good to go.

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.

2 participants