search

cnuernber / dtype-next

A Clojure library designed to aid in the implementation of high performance algorithms and systems.
Other
319 stars 18 forks source link

Fixed type hints in parallel-for #84

Closed Cortys closed 1 year ago

Cortys commented 1 year ago

Playing around a bit more with dtype, I found another minor issue: The type hints in parallel-for are incorrect; ^long in a syntax quote resolves to {:tag clojure.core/long} instead of {:tag 'long}. This is not recognized by the Clojure compiler as a valid type hint (context).

Normally, this does not cause any problems; the hint is simply ignored. However, when invoking the parallel-for macro from a Clerk notebook, the expanded form is analyzed via tools.analyzer.jvm. The invalid type hint causes an exception in the analyzer (ClassNotFoundException) and prevents the Clerk notebook from loading. While I think that this is mostly an issue with the way Clerk deals with such errors, I think fixing the error condition in dtype makes sense anyway.

Tests are still passing.

harold commented 1 year ago

Thanks, this is interesting. I wonder what Chris will think about this variation vs. calling (long ...) in the body of that function.

cnuernber commented 1 year ago

This is a good fix for sure - that bit of macro-foo I now know but I didn't then when I wrote that stuff. That specific function has been superceded by pgroups and upgroups in ham-fisted - something that perhaps should be noted in comments.