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

add size for column of Map #255

Merged
merged 1 commit into from
Feb 20, 2018

Conversation

ayoub-benali
Copy link
Contributor

@ayoub-benali ayoub-benali commented Feb 15, 2018

The curent implementation of size doesn't handle Map as it needs two types for key and value.

The following example leads to this error:

    import frameless.TypedDataset
    import frameless.functions.size
    case class Foo(a: Map[String, String])
    val ds = TypedDataset.create(List(Foo(Map("foo" -> "bar"))))
    ds.select(size(ds('a)))
no type parameters for method size: (column: frameless.TypedColumn[T,V[A]])(implicit evidence$1: frameless.functions.CatalystSizableCollection[V])frameless.TypedColumn[T,Int] exist so that it can be applied to arguments (frameless.TypedColumn[Foo,Map[String,String]])
[error]  --- because ---
[error] argument expression's type is not compatible with formal parameter type;
[error]  found   : frameless.TypedColumn[Foo,Map[String,String]]
[error]  required: frameless.TypedColumn[?T,?V[?A]]
[error]     ds.select(size(ds('a)))

This PR adds a size function to handle Map

@codecov-io
Copy link

codecov-io commented Feb 15, 2018

Codecov Report

Merging #255 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   96.56%   96.57%   +<.01%     
==========================================
  Files          51       51              
  Lines         874      875       +1     
  Branches       11       11              
==========================================
+ Hits          844      845       +1     
  Misses         30       30
Impacted Files Coverage Δ
...ain/scala/frameless/functions/UnaryFunctions.scala 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 652fa18...4192a6b. Read the comment docs.

@OlivierBlanvillain
Copy link
Contributor

LGTM, thanks!

@OlivierBlanvillain OlivierBlanvillain merged commit e2c01d5 into typelevel:master Feb 20, 2018
@imarios
Copy link
Contributor

imarios commented Feb 26, 2018

@ayoub-benali We already had a typeclass CatalystSizableCollection that does that. Did you try adding Map there and it didn't work?

@imarios
Copy link
Contributor

imarios commented Feb 26, 2018

hmm, maybe the fact that Map has two holes and all the other ones have one hole might have caused an issue. was that the case? It would work with any V[_] but maybe not with V[_,_]?

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.

4 participants