Compile error with nim 1.2.0

[hiteshjasani]

I'm in the process of trying to upgrade some code to Nim 1.2.0 and ran into some problems. So I tried running the NimData testcases under 1.2.0 and got similar errors.

$ nimble test
# no errors

$ nimble tests
[...]

/Users/hitesh/dev/NimData/src/nimdata.nim(529, 16) Error: type mismatch: got <DataFrame[system.int]>
but expected one of: 
macro collect(init, body: untyped): untyped
  first type mismatch at position: 2
  missing parameter: body

expression: collect(df)
stack trace: (most recent call last)
/private/var/folders/f4/gzyr8mcn5yv0jvdv5l47zygr0000gn/T/nimblecache/nimscriptapi.nim(165, 16)
/Users/hitesh/dev/NimData/nimdata_97051.nims(52, 12) testsTask
/Users/hitesh/dev/NimData/nimdata_97051.nims(47, 8) runTest
/Users/hitesh/.choosenim/toolchains/nim-1.2.0/lib/system/nimscript.nim(260, 7) exec
/Users/hitesh/.choosenim/toolchains/nim-1.2.0/lib/system/nimscript.nim(260, 7) Error: unhandled exception: FAILED: nim c --cc:gcc --verbosity:0 -r -d:travis -d:testNimData --outdir:bin tests/all.nim [OSError]
     Error: Exception raised during nimble script execution

[mratsim]

Nim added a collect macro that takes 2 argument and it seems like it causes trouble when a method has the same name: even though the types don't match with macros/procs Nim doesn't try to look for eventual methods overloads?

NimData/src/nimdata.nim

Lines 518 to 527 in 845fce2

	method collect*[T](df: DataFrame[T]): seq[T] {.base.} =
	## Collects the content of a ``DataFrame[T]`` and returns it as ``seq[T]``.
	result = newSeq[T]()
	let it = df.iter()
	for x in it():
	result.add(x)
	method collect*[T](df: CachedDataFrame[T]): seq[T] =
	## Specialized implementation
	result = df.data

[bluenote10]

Yes it is the infamous "macro with untyped args messes up all overloading" again (introduced by here).

Ironically NimData originally used the name df.toSeq() to convert a data frame into a seq, but this got broken when importing the toSeq from sequtils, because of an untyped macro. I then changed the name from toSeq to collect to avoid the name clash, but now the stdlib again introduces a macro that messes up the API. The work-around is simple: Either rename the function again, or use selective imports on sugar now everywhere, and NimData won't be compatible with the ubiquous import sugar. Two ugly options, which is why I couldn't motivate myself so far to do it.

What would you prefer?

[hiteshjasani]

I personally didn't use the sugar module much before, but with the addition of list comprehensions via the collect macro I can see it being much more useful.

My suggestion would be to rename it. Naming is hard so I'll help brainstorm:

• cache
• run
• rcache - run/resolve operations up to this point and cache result
• ruache - like rcache except a fake name that lowers chance of future collision
• resolve
• toValue
• eval
• unlazyify

[bluenote10]

Should be fixed in #60, I decided to go for import sugar except collect, because it is the minimal change, not affecting the API itself. If that turns out to be too annoying, a renaming can happen later.

Note that there may still be issues on devel coming from incompatibilities with plotting: SciNim/nim-plotly#63