Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

Vendorize StaticArrays.FixedSizeArrays #153

Merged
merged 4 commits into from
Nov 5, 2018

Conversation

aaronang
Copy link
Contributor

@aaronang aaronang commented Oct 2, 2018

This patch allows the JuliaArrays/StaticArrays.jl repository to deprecate StaticArrays.FixedSizeArrays without affecting GeometricTypes.jl.

Resolves #132.

I am very new to Julia and I am not sure if this patch makes any sense. I am looking forward to receiving constructive feedback 🙂

This patch allows the JuliaArrays/StaticArrays.jl repository to
deprecate StaticArrays.FixedSizeArrays without affecting
GeometricTypes.jl.
@rdeits
Copy link
Contributor

rdeits commented Oct 2, 2018

Thanks! This seems perfectly reasonable to me.

For clarity and proper attribution, could you please add a comment at the beginning of FixedSizeArrays.jl saying something like:

This code was copied from https://github.com/JuliaArrays/StaticArrays.jl which is written by Andy Ferris and released under the MIT "Expat" License: 

> Copyright (c) 2016: Andy Ferris.
>
> Permission is hereby granted, free of charge, to any person obtaining
> a copy of this software and associated documentation files (the
> "Software"), to deal in the Software without restriction, including
> without limitation the rights to use, copy, modify, merge, publish,
> distribute, sublicense, and/or sell copies of the Software, and to
> permit persons to whom the Software is furnished to do so, subject to
> the following conditions:
>
> The above copyright notice and this permission notice shall be
> included in all copies or substantial portions of the Software.
>
> THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
> CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

(which is just a copy of the license file from https://github.com/JuliaArrays/StaticArrays.jl/blob/master/LICENSE.md )

@aaronang
Copy link
Contributor Author

aaronang commented Oct 2, 2018

Done. Is there by the way anything we can do about the following warnings?

WARNING: Method definition isnan(StaticArrays.StaticArray{S, T, N} where N where T where S<:Tuple) in module FixedSizeArrays at /home/travis/.julia/packages/StaticArrays/Ze5H3/src/FixedSizeArrays.jl:60 overwritten in module FixedSizeArrays at /home/travis/build/JuliaGeometry/GeometryTypes.jl/src/FixedSizeArrays.jl:83.
WARNING: Method definition extrema(AbstractArray{T<:(StaticArrays.StaticArray{Tuple{N}, T, 1} where T where N), 1}) where {T<:(StaticArrays.StaticArray{Tuple{N}, T, 1} where T where N)} in module FixedSizeArrays at /home/travis/.julia/packages/StaticArrays/Ze5H3/src/FixedSizeArrays.jl:77 overwritten in module FixedSizeArrays at /home/travis/build/JuliaGeometry/GeometryTypes.jl/src/FixedSizeArrays.jl:100.
WARNING: Method definition minimum(AbstractArray{T<:(StaticArrays.StaticArray{Tuple{N}, T, 1} where T where N), 1}) where {T<:(StaticArrays.StaticArray{Tuple{N}, T, 1} where T where N)} in module FixedSizeArrays at /home/travis/.julia/packages/StaticArrays/Ze5H3/src/FixedSizeArrays.jl:81 overwritten in module FixedSizeArrays at /home/travis/build/JuliaGeometry/GeometryTypes.jl/src/FixedSizeArrays.jl:104.
WARNING: Method definition maximum(AbstractArray{T<:(StaticArrays.StaticArray{Tuple{N}, T, 1} where T where N), 1}) where {T<:(StaticArrays.StaticArray{Tuple{N}, T, 1} where T where N)} in module FixedSizeArrays at /home/travis/.julia/packages/StaticArrays/Ze5H3/src/FixedSizeArrays.jl:84 overwritten in module FixedSizeArrays at /home/travis/build/JuliaGeometry/GeometryTypes.jl/src/FixedSizeArrays.jl:107.

@rdeits
Copy link
Contributor

rdeits commented Oct 2, 2018

We shouldn't include those definitions in GeometryTypes as it's technically type piracy. They have nothing to do with FixedSizeArrays, so they should probably just be defined in StaticArrays.jl or not defined at all.

@andyferris will the methods here: https://github.com/JuliaArrays/StaticArrays.jl/blob/86097997bd846f0b9f4331cd55ba8a233375173b/src/FixedSizeArrays.jl#L59-L85 be preserved when you deprecate and remove FixedSizeArrays?

@aaronang
Copy link
Contributor Author

aaronang commented Oct 2, 2018

@rdeits I removed the definitions that from FixedSizeArrays. I assume we will have to wait for @andyferris's reply before can merge it?

Copy link

@andyferris andyferris left a comment

Choose a reason for hiding this comment

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

Thanks for taking this step.

This code was copied from https://github.com/JuliaArrays/StaticArrays.jl which
is written by Andy Ferris and released under the MIT "Expat" License:

> Copyright (c) 2016: Andy Ferris.

Choose a reason for hiding this comment

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

You have my permission to drop the license and include the code in GeometryTypes. I'm assuming @SimonDanisch will similarly agree.

Copy link
Member

Choose a reason for hiding this comment

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

Sure :)

@andyferris
Copy link

We shouldn't include those definitions in GeometryTypes as it's technically type piracy. They have nothing to do with FixedSizeArrays, so they should probably just be defined in StaticArrays.jl or not defined at all.

There were always a couple small definitions in FixedSizedArrays which went beyond the typical AbstractArray interface (one explicit design goal for StaticArrays was to follow the AbstractArray interface a strictly as feasibly possible so that they can be used interchangeably). Personally, I'd recommend following what's in Base and StaticArrays and for whatever is missing you either add here as a geometric concept, or make a PR to Base or StaticArrays for generic concepts. For example, I still feel slightly guilty about the length-2 "cross" product... :)

Andy Ferris:

> You have my permission to drop the license and include the code in GeometryTypes. I'm assuming @SimonDanisch will similarly agree.
@aaronang
Copy link
Contributor Author

aaronang commented Oct 8, 2018

I removed the attribution based on @andyferris's feedback. Please let me know if I did it incorrectly 🙂

@c42f
Copy link
Member

c42f commented Oct 23, 2018

@rdeits I had a look at the functions in https://github.com/JuliaArrays/StaticArrays.jl/blob/86097997bd846f0b9f4331cd55ba8a233375173b/src/FixedSizeArrays.jl#L59-L85. IMO most of these should not be preserved in StaticArrays:

  • There is no canonical notion of ordering in a multidimensional space, so we should not provide maximum, minimum or extrema. extrema is really calculating the axis aligned bounding box, and should be renamed appropriately and implemented in GeometryTypes if needed.
  • Base.isnan does not apply to arrays (an array can contain NaNs but cannot be a NaN), and we should follow the lead there. any(isnan, v) appears to generate efficient code for that.

We might think about retaining unit in StaticArrays proper, though I'd want to rename it to something less generic, perhaps unitvec.

@SimonDanisch SimonDanisch merged commit 6d54758 into JuliaGeometry:master Nov 5, 2018
@SimonDanisch
Copy link
Member

Thank you, this seems to work fine :)

@aaronang aaronang deleted the issue-132 branch November 5, 2018 17:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants