-
Notifications
You must be signed in to change notification settings - Fork 152
Code clean and reuse for constructor.
#1016
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
Conversation
c7c5585 to
90351f2
Compare
|
|
||
| const allowable_ambiguities = VERSION ≥ v"1.7" ? 60 : | ||
| VERSION ≥ v"1.6" ? 61 : error("version must be ≥1.6") | ||
| const allowable_ambiguities = VERSION ≥ v"1.7" ? 10 : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎆
c8eaf3b to
294f2ef
Compare
294f2ef to
c1fd9b3
Compare
71cd505 to
aee1ace
Compare
17e8b43 to
2fd8abb
Compare
hyrodium
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the changes seem great! I have added some comments.
690150f to
02c1449
Compare
|
Is this ready to merge? |
Similar to `similar_type`, but not fallback to `SArray`. It is used to pick the most `concrete` constructor with `size`, `eltype` and `ndims` defined.
And fix for input with non-1 based axes.
This also enable auto type promotion.
With `construct_type`, there's no need to keep all these dispatches. This also fix empty construction for `S/MVector`, and remove most of the ambiguities.
1. remove `StaticSquareMatrix` (`StaticMatrix{N,N}` should be shorter and clear enough)
2. Add missing Test.
And convert comments to docstring.
|
I've added a warning to |
|
Sorry for the late review. The rest of the code looks great to me. |
and typo fix.
Seperated from #1009.
This PR aims to unify the implement of
StaticArray's constructor with a new methodconstruct_type.It returns a
concreteconstructor based onsrc <: Union{Tupe, StaticArray, AbstractArray}and the target typeSA(if possible).e.g.
Thus, we only need to mannually define the constructor with precise
size,eltypeandndims.Unneeded dispatch has been removed.
Edit1: Since 3rd party
StaticArraymight prefer previous design, the defaultconstruct_typereturns the inputSAdirectly to keep compatibility.Edit2:
construct_typeforFieldArrayhas been implemented. NowFieldArray's default constructor support eltype promotion.This PR also restrict the input wrapping within constructor.
If multiple arguments are provided, all of them should be treated as the result's elements, and re-wrapping is not allowed anymore.
For example
Close #809.
Close #911. (fix constructor missing)