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.

Sign up for GitHub

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

Multiple updates #1

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+77 −21
Open

Multiple updates #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bc1e67b
Comment broken tests
goretkin Feb 6, 2021
ebc3a71
Define and test `Base.map`
goretkin Feb 6, 2021
e241350
Remove type restriction `::Function`
goretkin Feb 6, 2021
c4ff1ef
Re-enable `hasmethod` functionality and tests
goretkin Feb 10, 2021
1c16333
Copy template for Github Actions CI
goretkin Feb 10, 2021
40961d3
Copy `ci.yml` from `FixArgs.jl`
goretkin Feb 10, 2021
b94d7a2
Restore some of original `ci.yml`
goretkin Feb 10, 2021
26fc307
Fix test in docstring
goretkin Feb 10, 2021
e31680d
Prevent doctests from breaking once 1.6 is released
goretkin Feb 10, 2021
ffb20c6
Update docs
goretkin Feb 10, 2021
6cc32fd
Add compat bound and test minimum version
goretkin Feb 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
name: CI
on:
- push
- pull_request
jobs:
test:
name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
version:
- '1.2' # minimum Julia version that the package supports.
- '1' # Leave this line unchanged. '1' will automatically expand to the latest stable 1.x release of Julia.
- 'nightly'
os:
- ubuntu-latest
arch:
- x64
steps:
- uses: actions/checkout@v2
- uses: julia-actions/setup-julia@v1
with:
version: ${{ matrix.version }}
arch: ${{ matrix.arch }}
- uses: actions/cache@v1
env:
cache-name: cache-artifacts
with:
path: ~/.julia/artifacts
key: ${{ runner.os }}-test-${{ env.cache-name }}-${{ hashFiles('**/Project.toml') }}
restore-keys: |
${{ runner.os }}-test-${{ env.cache-name }}-
${{ runner.os }}-test-
${{ runner.os }}-
- uses: julia-actions/julia-buildpkg@latest
- uses: julia-actions/julia-runtest@latest
- uses: julia-actions/julia-processcoverage@v1
- uses: julia-actions/julia-uploadcodecov@latest
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
docs:
name: Documentation
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: julia-actions/setup-julia@v1
with:
version: '1.5' # 1.6 will break doctests due to type aliases, until the tests are updated
- run: |
julia --project=docs -e '
using Pkg
Pkg.develop(PackageSpec(path=pwd()))
Pkg.instantiate()'
- run: julia --project=docs docs/make.jl
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
3 changes: 3 additions & 0 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ name = "FrankenTuples"
uuid = "7e584817-dab4-53a9-9a51-4037a36b0ad0"
version = "0.0.1"

[compat]
julia = "1.2"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

Expand Down
2 changes: 1 addition & 1 deletion docs/src/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ Base.first
Base.tail
Base.empty
Base.eltype
Base.map
```

## Additional Methods

These are some additional ways to use `FrankenTuple`s.
The most interesting of these is perhaps `hasmethod`, which permits looking for methods
that have particular keyword arguments.
This is not currently possible with the generic method in Base.

```@docs
Base.hasmethod
Expand Down
31 changes: 11 additions & 20 deletions src/FrankenTuples.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ julia> f(x::Int; y=3, z=4) = x + y + z;
julia> hasmethod(f, FrankenTuple{Tuple{Int},(:y,)})
true

julia> hasmethod(f, FrankenTuple{Tuple{Int},(:a,)) # no keyword `a`
julia> hasmethod(f, FrankenTuple{Tuple{Int},(:a,)}) # no keyword `a`
false

julia> g(; a, b, kwargs...) = +(a, b, kwargs...);
Expand All @@ -312,30 +312,14 @@ true
function Base.hasmethod(f::Function,
::Type{FrankenTuple{T,names,NT}};
world=typemax(UInt)) where {T<:Tuple,names,NT<:Tuple}
hasmethod(f, T) || return false
m = which(f, T)
Base.max_world(m) <= world || return false
kws = Base.kwarg_decl(m, Core.kwftype(typeof(f)))
isempty(kws) === isempty(names) || return false
for (i, k) in enumerate(kws)
if endswith(String(k), "...")
deleteat!(kws, i)
return issubset(kws, names)
end
end
issubset(names, kws)
hasmethod(f, T, names; world=world)
end
function Base.hasmethod(f::Function,
::Type{FrankenTuple{T,names}};
world=typemax(UInt)) where {T<:Tuple,names}
NT = Tuple{Iterators.repeated(Any, length(names))...}
hasmethod(f, FrankenTuple{T,names,NT}; world=world)
end
function Base.hasmethod(f::Function,
::Type{FrankenTuple{T,(),Tuple{}}};
world=typemax(UInt)) where {T<:Tuple}
hasmethod(f, T; world=world)
end

"""
ftuple(args...; kwargs...)
Expand Down Expand Up @@ -418,7 +402,14 @@ julia> ftcall(mapreduce, ftuple(abs2, -, 1:4; init=0))
-30
```
"""
ftcall(f::Function, ft::FrankenTuple) = f(Tuple(ft)...; NamedTuple(ft)...)
ftcall(f::Function, ft::FrankenTuple{Tuple{},(),Tuple{}}) = f()
ftcall(f, ft::FrankenTuple) = f(Tuple(ft)...; NamedTuple(ft)...)
ftcall(f, ft::FrankenTuple{Tuple{},(),Tuple{}}) = f()

# NOTE: this method signature makes sure we don't define map(f)
function Base.map(f, ft::FrankenTuple, fts::FrankenTuple...)
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this definition of map really jives with what map does for other collections, since iteration of FrankenTuples provides the elements in sequence rather than treating the named and unnamed parts separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I think that might be okay. I wonder if perhaps you expect isequal(map(f, c), map(f, collect(c))) (which I kind of expect as well). However that's already not the case for NamedTuple (second test below):

using FrankenTuples

function _test_map(f, c)
    (map(f, c), map(f, collect(c)))
end

@show _test_map(sqrt, (1, 4))
@show _test_map(sqrt, (a=9, b=16))
@show _test_map(sqrt, @ftuple (1, 4, a=9, b=16))
_test_map(sqrt, (1, 4)) = ((1.0, 2.0), [1.0, 2.0])
_test_map(sqrt, (a = 9, b = 16)) = ((a = 3.0, b = 4.0), [3.0, 4.0])
_test_map(sqrt,  @ftuple((1, 4, a = 9, b = 16))) = (FrankenTuple((1.0, 2.0), (a = 3.0, b = 4.0)), [1.0, 2.0, 3.0, 4.0])

I think the right "invariant" is

function test_map_collect_commute(f, c)
    isequal(
        collect(map(f, c)),
        map(f, collect(c))
    )
end

@show test_map_collect_commute(sqrt, (1, 4))
@show test_map_collect_commute(sqrt, (a=9, b=16))
@show test_map_collect_commute(sqrt, @ftuple (1, 4, a=9, b=16))

and it gives

test_map_collect_commute(sqrt, (1, 4)) = true
test_map_collect_commute(sqrt, (a = 9, b = 16)) = true
test_map_collect_commute(sqrt, @ftuple((1, 4, a = 9, b = 16))) = true

Copy link
Contributor Author

@goretkin goretkin Jul 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@ararslan What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see that you've just recently cherry picked some of the commits from this PR, and addressed some of the other commits too. Great!

Is map the only thing that this PR would add, then?

Copy link
Owner

Choose a reason for hiding this comment

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

Yep, just wanted to get the repo into a better state then discuss/ruminate on map separately. I made sure to preserve your authorship for things you fixed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed, and I appreciate it :)

t = map(f, Tuple(ft), map(Tuple, fts)...)
nt = map(f, NamedTuple(ft), map(NamedTuple, fts)...)
FrankenTuple(t, nt)
end

end # module
6 changes: 6 additions & 0 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,9 @@ h(x::String; a, kwargs...) = x * a

@test !hasmethod(f, FrankenTuple{Tuple{Int},(:y,)}, world=typemin(UInt))
end

@testset "map" begin
@test FrankenTuple((11,22), (x=33,y=44)) == map(+, FrankenTuple((1,2), (x=3,y=4)), FrankenTuple((10, 20), (x=30,y=40)))
@test_throws Exception map(+, FrankenTuple((1,2), (x=3,y=4)), FrankenTuple((10, 20), (x=30,)))
@test_throws Exception map(+, FrankenTuple((1,2), (x=3,y=4)), FrankenTuple((10,), (x=30,y=40)))
end