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

Set default base for round to 10 #537

Merged
merged 4 commits into from
Aug 22, 2018
Merged

Conversation

jmkuhn
Copy link
Contributor

@jmkuhn jmkuhn commented Apr 26, 2018

No description provided.

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Apr 26, 2018

The same issue exists for floor, ceil, trunc. I'll take a look at those now.

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Apr 26, 2018

The AppVeyor build failure is fixed in Julia version 0.7.0-DEV.4942.

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Jun 6, 2018

Fixes #567.

trunc(x; digits = 0, base = 10) = Base.trunc(x, digits, base)
floor(x; digits = 0, base = 10) = Base.floor(x, digits, base)
ceil(x; digits = 0, base = 10) = Base.ceil(x, digits, base)
function round(x; digits = nothing, sigdigits = nothing, base = 10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these should be constants?

@martinholters
Copy link
Member

Should probably also test cases where base is given but digits is not, and where neither is given.

@martinholters
Copy link
Member

Error During Test at /Users/travis/build/JuliaLang/Compat.jl/test/runtests.jl:1467
  Test threw exception MethodError(round, (π = 3.1415926535897..., RoundingMode{:Nearest}()), 0xffffffffffffffff)
  Expression: Compat.round(pi) == 3.0
  MethodError: no method matching round(::Irrational{:π}, ::RoundingMode{:Nearest})
  Closest candidates are:
    round(::Irrational, ::RoundingMode) at irrationals.jl:128

???

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Jun 8, 2018

I'm looking at the Irrational issue now.

@jmkuhn
Copy link
Contributor Author

jmkuhn commented Jun 8, 2018

julia> Compat.round(1.2, RoundNearest)
ERROR: MethodError: no method matching round(::Float64, ::RoundingMode{:Nearest}, ::Int64)
Closest candidates are:
  round(::Real, ::Integer, ::Integer) at floatfuncs.jl:152
  round(::Float64) at float.jl:352
  round(::Complex{#s45} where #s45<:AbstractFloat, ::RoundingMode{MR}, ::RoundingMode{MI}) where {MR, MI} at complex.jl:885

Need to add additional methods to Compat.round.

darwindarak added a commit to darwindarak/PotentialFlow.jl that referenced this pull request Jun 30, 2018
`round` in v0.6 does not take in keyword arguments
Have to add a `base` keyword argument now while waiting for
JuliaLang/Compat.jl#537
darwindarak added a commit to darwindarak/PotentialFlow.jl that referenced this pull request Jul 1, 2018
`round` in v0.6 does not take in keyword arguments
Have to add a `base` keyword argument now while waiting for
JuliaLang/Compat.jl#537
@martinholters
Copy link
Member

This has gone a bit stale, so I'll close/reopen to rerun CI. If that passes, should be good to go.

@martinholters
Copy link
Member

martinholters commented Jul 18, 2018

Test failures are because on 0.7, we have:

julia> round(pi, base=10)
ERROR: MethodError: no method matching round(::Irrational{:π}, ::RoundingMode{:Nearest})
Closest candidates are:
  round(::Irrational, ::RoundingMode) at irrationals.jl:137
  round(::Real, ::RoundingMode; digits, sigdigits, base) at floatfuncs.jl:127
  round(::Number, ::Any) at deprecated.jl:53

(Already noted above; I had assumed this had been fixed in the meantime.) Ironically:

julia> round(pi, RoundingMode{:Nearest}())
3.0

The method error above is explicitly thrown in floatfuncs.jl:

# if we hit this method, it means that no `round(x, r)` method is defined
_round(x::Real, r::RoundingMode, digits::Nothing, sigdigits::Nothing, base) = throw(MethodError(round, (x,r)))

That's a genuine issue in base. (EDIT: filed as JuliaLang/julia#28160.) To get this PR going, I'd propose to just replace pi with 3.14159. EDIT: Nonsense, this is not specific to Irrationals, so using a float does not help.

@martinholters
Copy link
Member

This has gone a bit stale, so I'll close/reopen to rerun CI. If that passes, should be good to go.

(Yes, I've said this before.)

martinholters added a commit that referenced this pull request Oct 13, 2019
* Bump required Julia version to 1.0

* Remove compatibility support code for:
  * `at-__MODULE__` (from #363)
  * `devnull`, `stdin`, `stdout`, and `stderr` from #499
  * `at-nospecialize` (from #385 and #409)
  * `isabstracttype` and `isconcretetype` (from #477)
  * `invokelatest` from #424
  * array-like access to `Cmd` from #379
  * `Val(n)` and `ntuple`/`reshape` with `Val` from #381 and #399
  * `logdet(::Any)` fallback from #382
  * `chol(::UniformScaling)` from #382
  * `pushfirst!`, `popfirst!` from #444
  * `fieldcount` from #386
  * `read(obj, ::Type{String})` from #385 and #580
  * `InexactError`, `DomainError`, and `OverflowError` constructors from #393
  * `corrected` kw arg to `cov` from #401
  * `adjoint` from #401
  * `partialsort` from #401
  * `pairs` from #428
  * `AbstractRange` from #400
  * `rtoldefault` from #401
  * `Dates.Period` rounding from #462
  * `IterativeEigensolvers` from #435
  * `occursin` from #520
  * `Char` concatenation from #406
  * `BitSet` from #407
  * `diagm` and `spdiagm` with pairs from #408
  * `Array` c'tors from `UniformScaling` from #412 and #438
  * `IOContext` ctor taking pairs from #427
  * `undef` from #417 and #514
  * `get` on `ENV` from #430
  * `ComplexF...` from #431
  * `tr` from #614
  * `textwidth` from #644
  * `isnumeric` from #543
  * `AbstractDict` from #435
  * `axes` #435 and #442
  * `Nothing` and `Cvoid` from #435
  * `Compat.SuiteSparse` from #435
  * `invpermute!` from #445
  * `replace` with a pair from #445
  * `copyto!` from #448
  * `contains` from #452
  * `CartesianIndices` and `LinearIndices` from #446, #455, and #524
  * `findall` from #466 (and #467).
  * `argmin` and `argmax` from #470, #472, and #622
  * `parentmodule` from #461
  * `codeunits` from #474
  * `nameof` from #471
  * `GC` from #477
  * `AbstractDisplay` from #482
  * `bytesavailable` from #483
  * `firstindex` and `lastindex` from #480 and #494
  * `printstyled` from #481
  * `hasmethod` from #486
  * `objectid` from #486
  * `Compat.find*` from #484 and #513
  * `repr` and `showable` from #497
  * `Compat.names` from #493 and #505
  * `Compat.round` and friends #500, #530, and #537
  * `IOBuffer` from #501 and #504
  * `range` with kw args and `LinRange` from #511
  * `cp` and `mv` from #512
  * `indexin` from #515
  * `isuppercase` and friends from #516
  * `dims` and `init` kwargs from #518, #528, #590, #592, and #613
  * `selectdim` from #522 and #531
  * `repeat` from #625
  * `fetch(::Task)` from #549
  * `isletter` from #542
  * `isbitstype` from #560
  * `at-cfunction` from #553 and #566
  * `codeunit` and `thisind` and friends from #573
  * `something` from #562
  * `permutedims` from #582
  * `atan` from #574
  * `split` and `rsplit` from #572
  * `mapslices` from #588
  * `floatmin` and `floatmax` from #607
  * `dropdims` from #618
  * required keyword arguments from #586
  * `CartesianRange` in `at-compat` from #377
  * `finalizer` from #416
  * `readline`, `eachline`, and `readuntil` from #477, #541, and #575
  * curried `isequal`, `==`, and `in` from #517
  * `Some` from #435 and #563
  * `at-warn` and friends from #458

* Remove old deprecations

* Deprecate:
  * `Compat.Sockets` from #545 and #594
  * `TypeUtils` from #304
  * `macros_have_sourceloc` from #355
  * `Compat.Sys` from #380, #433, and #552
  * `Compat.MathConstants` from #401
  * `Compat.Test`, `Compat.SharedArrays`, `Compat.Mmap`, and `Compat.DelimitedFiles` from #404
  * `Compat.Dates` from #413
  * `Compat.Libdl` from #465 (and #467)
  * `AbstractDateTime` from #443
  * `Compat.Printf` from #435
  * `Compat.LinearAlgebra` from #463
  * `Compat.SparseArrays` from #459
  * `Compat.Random` from #460, #601, and #647
  * `Compat.Markdown` from #492
  * `Compat.REPL` from #469
  * `Compat.Serialization` from #473
  * `Compat.Statistics` from #583
  * `Fix2` from #517
  * `Compat.Base64` from #418
  * `Compat.Unicode` from #432 and #507
  * `notnothing` from #435 and #563
  * `Compat.IteratorSize` and `Compat.IteratorEltype` from #451
  * `enable_debug(::Bool)` from #458
  * `Compat.Distributed` from #477
  * `Compat.Pkg` from #485
  * `Compat.InteractiveUtils` from #485
  * `Compat.LibGit2` from #487
  * `Compat.UUIDs` from #490
  * `Compat.qr` from #534
  * `Compat.rmul!` from #546
  * `Compat.norm` abd friends from #577

* Remove obsolete README entry, missed in #385

* Remove obsolete tests (e.g. missed in #372)

* Remove obsolete `VERSION` conditionals and some minor clean-up
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