Implement some-> macro

[wrobell]

Thread head first through the rest of the forms, but short-circuit if a form returns None.

This is an attempt to progress with #5 ticket.

[Kodiologist]

You're assuming that -> exists in the scope where some-> expands:

$ echo '(require hyrule [some->]) (some-> 1 (+ 2))' | hy
NameError: name 'hyx_XhyphenHminusXXgreaterHthan_signX' is not defined

So you should refer to -> as hy.R.hyrule.-> instead.

steps and steps_none are used only once, so you can inline them. In fact, you can provide both parts in one lfor and skip the zip entirely.

"un-even" is better written "odd".

A test of short-circuiting should check that no side-effects occur of forms that are supposed to be skipped. I typically do this by trying to append to a list and then checking that the list is still empty.

The intended property of some-> that the thread ends when one of the arguments itself evaluates to None, as in (setv x None) (some-> 1 x (+ 2)), appears to be untested.

[wrobell]

Thanks for the feedback.

BTW. Do you think it would make sense to extract the processing of a form (use of _dotted, and if statement for expression) into separate macro, and then reuse it in both -> and some->?

[Kodiologist]

I'm not sure. You could try it and see if the result is neater. Subroutines of macros are generally better written as functions than macros, though.

[wrobell]

The intended property of some-> that the thread ends when one of the arguments itself evaluates to None, as in (setv x None) (some-> 1 x (+ 2)), appears to be untested.

From https://clojuredocs.org/clojure.core/some-%3E (emphasis mine)

threads it into the first form (via ->), and when that result is not nil, through the next

(setv x None) would be equivalent to some-> 1 None (+ 2)) and would result in TypeError: 'NoneType' object is not callable, which is expected result?

BTW. There is the following test, which I have extended a bit (commit 9b30918)

(defn test-threading-some-circuit []
  (defn ret_none [a b] None)
  (assert (is (some-> 1 (+ 2) (ret_none 3) (* 3)) None)))

[wrobell]

Feedback tasks

• use hy.R.hyrule.-> (9b30918)
• s/uneven/odd/ (9b30918)
• side effects check in short-circuit tests (maybe ec9317e)
• what if None is passed instead of a function? (see Implement some-> macro #87 (comment))
• inline steps_none within steps (3be1f38)
• update NEWS file (57b33cb)
• refrerence some-> in the manual (57b33cb)

[wrobell]

steps and steps_none are used only once, so you can inline them. In fact, you can provide both parts in one lfor and skip the zip entirely.

Originally I created steps as a tuple of (form, None). This was hard to follow.

What if we get rid of zip with

  (setv steps (gfor node args 
                     ...))
  ; interleave each form with None to use them with `cond` macro below
  (setv steps (gfor v steps #(v None)))

Please see commit 9b30918 (and a small fix 2a35619).

[wrobell]

A test of short-circuiting should check that no side-effects occur of forms that are supposed to be skipped. I typically do this by trying to append to a list and then checking that the list is still empty.

IMHO, this is better done with mocking, please see commit ec9317e.

[Kodiologist]

What if we get rid of zip with…

Still seems overcomplicated to me. See my new commit.

Otherwise, looks good. I guess I misunderstood how some-> is supposed to work. I didn't even know Python's built-in mock library existed, so, good to know. Add a NEWS item and update the manual and this should be done.

[Kodiologist]

And don't forget to add yourself to AUTHORS.

[wrobell]

Add a NEWS item and update the manual and this should be done.

Done via commit 57b33cb.

Anything else to correct, let me know please. Otherwise, for summary, please see #87 (comment).

[davidahern]

Probably i shouldn't have permission to do this

[Kodiologist]

Good point. That should be fixed now.

[wrobell]

@Kodiologist @scauligi i have asked my friend to approve as it seems the settings are wide open. i suspect this is not intended?

[Kodiologist]

Yes, I was under the impression that GitHub actually didn't let you restrict reviewing privileges, but now, at least, it does.