Cosmetic changes #72
Closed
Cosmetic changes #72
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some cosmetic changes are made related to:
- **Inner constructor.** Now, `Poly` is defined only for `SymbolLike`
objects, *i.e.*, `Char`, `AbstractString` and `Symbol`s,
- **Poly from variables.** Since `Poly("x")` could have resulted in
arguments related to what the default constructor should do (maybe a
default behaviour of constructing a `zero` polynomial with the given
variable is needed), I dropped my previous changes and extended
e29591d to include `SymbolLike` objects for `variable`. Also, an outer
constructor for `Number`s is added for convenience,
- [x] Fixes: #71.
- **Version checks.** Dropped `VERSION >= v"0.4"` checks as the support
for v"0.3" has already been dropped,
- **Base overloads.** Moved all implicitly overloaded base functions to
the import section on top.
I hope you would agree with the changes.
jverzani
reviewed
Nov 30, 2016
| end | ||
|
|
||
| @compat Poly{T<:Number}(a::Vector{T}, var::Union{AbstractString,Symbol,Char}=:x) = Poly{T}(a, var) | ||
| Poly(n::Number = zero(n), var::SymbolLike = :x) = Poly([n, zero(n)], var) | ||
|
|
There was a problem hiding this comment.
Are degree 0 polynomials so common to construct that we need a new constructor?
jverzani
reviewed
Nov 30, 2016
| copy(p::Poly) = Poly(copy(p.a), p.var) | ||
|
|
||
| zero{T}(p::Poly{T}) = Poly([zero(T)], p.var) | ||
| zero{T}(p::Poly{T}) = Poly(T[], p.var) |
There was a problem hiding this comment.
Doesn't this rely on an implementation detail whereas the other doesn't? I don't quite understand the motivation here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some cosmetic changes are made related to:
Polyis defined only forSymbolLikeobjects, i.e.,
Char,AbstractStringandSymbols,Poly("x")could have resulted inarguments related to what the default constructor should do (maybe a
default behaviour of constructing a
zeropolynomial with the givenvariable is needed), I dropped my previous changes and extended
e29591d to include
SymbolLikeobjects forvariable. Also, an outerconstructor for
Numbers is added for convenience,Poly("x")#71.VERSION >= v"0.4"checks as the supportfor v"0.3" has already been dropped,
the import section on top.
I hope you would agree with the changes.
Cc: @jverzani, @Keno