Virtue SKILL and Python Library

Hi,

This is a nice idea, I wish I could do the same but unfortunately the code I develop is not my propriety...

I looked at your source code and I have seen some things I believe you could improve :

Firstly, in your "packages", you recreate a DPL at the end of each let, you could get rid of rewriting the name of all the functions you define by using environments instead

;; Equivalent of what you do
package = (let nil
  (defun a nil nil)
  (defun b nil nil)
  `(nil a ,a b ,b)
   )

;; Could be
package = (let nil
  (defun a nil nil)
  (defun b nil nil)
  (theEnvironment)
  )

package->??
(((a funobj@0x3dafd368) 
    (b funobj@0x3dafd390)
    )

This function made me tick:

procedure(assocKeys(assocList "l") let(((out nil))
    "Returns all the keys in the specified association list"
      foreach(item assocList
          out = tconc(out car(item)))
      out
  ))

When you put a let as the second procedure argument instead of the docstrings you write rigorously, you bypass the native (and LISP conventional) docstring mechanism which is unfortunate if later you want to automate the documentation generation in SKILL for instance

Also you do (let ((out nil)) ... ) which is equivalent to (let (out) ... ) why be so meticulous when you could be lazy

You use tconc which is a nice idea because it is an underused and powerful function, except it makes no sense in your case.
You are already looping over your list: foreach mapcar or mapcar only will do the job for you.
Also you don't return the car of your tconc structure so you are not exactly returning what your docstring is describing

;; The following solutions are completely equivalent:
procedure(assocKeys(assocList "l")
    "Returns all the keys in the specified association list"
    foreach(mapcar item assocList
          (car item)
  ))

procedure(assocKeys(assocList "l")
    "Returns all the keys in the specified association list"
    mapcar( 'car assocList)
  )

The following function for instance could make a much better use of tconc :

procedure(uniqueListOrdered(in "l") let((item out noAdd)
    "Returns a list of items of the unique elements from the input list 
     while keeping the order between the first unique elements"
      out = '()
      foreach(item in
          noAdd = member(item out)
          unless(noAdd
              out = append(out list(item))
          ) 
      )
  out
  ))

;; Way faster using tconc as append rebrowse your list every time you loop in foreach
procedure(uniqueListOrdered(in "l")
    "Returns a list of items of the unique elements from the input list 
     while keeping the order between the first unique elements"
    let(((out tconc(nil nil)))
      foreach(item in
          unless(member(item cdar(out))
              tconc(out item)
          )
          )
  ;; Return list instead of tconc structure
  cdar(out)
  ))

I haven't had time to investigate further but this thanks for your share

Hope this helps

Cheers

Aurel

AurelBuche said:

package = (let nil
  (defun a nil nil)
  (defun b nil nil)
  (theEnvironment)
  )

One downside of this approach is it means that everything in the lexical scope then becomes visible via the environment object; you might want to have some internal functions and scoped variables, but this approach makes them all visible. Using a DPL (which could also be a table if you have a lot of functions to export) gives you the freedom to expose some but not all functions.

I didn't review the package in any detail or the rest of Aurélien's comments above, but thanks Curtismafor sharing this.

Andrew

I thought about that but I figured that as a final user there are less reason to hide functions and also packages prevent reusing names in the same namespace

If necessary a solution like this could do the job:

(defmacro make_package (name vars @rest body)
  `(let ,vars
     ,@body
      (set ',name
        (cons nil
          (foreach mapcan sym (theEnvironment)->?
            (unless (equal "_" (substring sym 1 1))
              (list sym (get (theEnvironment) sym))
              )))
        (schemeTopLevelEnv)
        )))

make_package( List (a (b 12))
  procedure(fun0(a)
    a)
  procedure(fun1()
    t)
  procedure(_hidden_fun()
    t)
  )

List->??
(b 12 a nil fun1
    funobj@0x3d51e160 fun0 funobj@0x3d51e138
)

I thought about that but I figured that as a final user there are less reason to hide functions and also packages prevent reusing names in the same namespace

If necessary a solution like this could do the job:

(defmacro make_package (name vars @rest body)
  `(let ,vars
     ,@body
      (set ',name
        (cons nil
          (foreach mapcan sym (theEnvironment)->?
            (unless (equal "_" (substring sym 1 1))
              (list sym (get (theEnvironment) sym))
              )))
        (schemeTopLevelEnv)
        )))

make_package( List (a (b 12))
  procedure(fun0(a)
    a)
  procedure(fun1()
    t)
  procedure(_hidden_fun()
    t)
  )

List->??
(b 12 a nil fun1
    funobj@0x3d51e160 fun0 funobj@0x3d51e138
)

Andrew and Aurel:

I thought about using a table for some packages that have a lot of functions.  While it would be faster I didn't like that there would be inconsistencies accessing packages if the two methods were mixed.  I also liked that the package function access was a different operator than package selection 

e.g. 

Import["Package"]["DoSomething"]

is not as clear that DoSomething is a function

Import["Package"]->DoSomething

I use the SKILL IDE and debugger.  When an error is caught and there is a macro around the error's code, the debugger will not point to the correct line in the code.  This makes it harder to debug.  I used to use a different test framework that used macros and suffered from this problem.

-Curtis

Hi Curtis,

Note that if a table is used where the key is a symbol then using Import["Package"]['DoSomething] and Import["Package"]->DoSomething both do the same thing...

So you can choose to optimise implementation internally without needing to change the external interface.

BTW on your other reponse, it seems a little odd to have to revert to using the underscore convention for internal functions when a major benefit lexically scoped functions is that you can control the visibility of them and truly hide them rather than relying on naming conventions.

Andrew

Note that if a table is used where the key is a symbol then using Import["Package"]['DoSomething] and Import["Package"]->DoSomething both do the same thing...

So you can choose to optimise implementation internally without needing to change the external interface.

Yeah, I noticed that over the weekend.  I'll probably end up doing that for some of the larger packages such as the "Str" package.

BTW on your other reponse, it seems a little odd to have to revert to using the underscore convention for internal functions when a major benefit lexically scoped functions is that you can control the visibility of them and truly hide them rather than relying on naming conventions.

Yeah, I agree.  I think the explicit export DPL list or table setup also makes the public interface clear.  It would be nice not to have to specify both the symbol and the value, but I haven't found a way to do that yet.

However, on the test runner side, it would be good to use the environment to find the test functions without selecting them individually in some cases to make sure all the test functions in a file were run.  There could still be the option to setup an explicit list of them.

Curtis,

A simple macro can be used to ease doing the export, which avoids having to give each name twice. For example:

(defmacro PkgExport (@rest _functions)
  `(let ((_pkgTable (makeTable 'pkgTable nil)))
     ,@(foreach mapcar _func _functions
                `(setarray _pkgTable (quote ,_func) ,_func))
     _pkgTable))

(setq myPkg
      (let ((data (makeTable 'storage nil)))
        (defun disp (msg)
          (printf "%s" msg))
        (defun set (key value)
          (disp "setting entry in storage\n")
          (setarray data key value))
        (defun get (key)
          (disp "getting entry from storage\n")
          (arrayref data key))
        ; simple way of exporting everything you want
        (PkgExport set get)
        )
      )

The PkgExport macro creates a table and populates it with the keys and function objects that you choose. The example at the bottom is just showing a simple usage of it to show the principle. BTW, the variables in the PkgExport macro have intentionally used names beginning with underscore to try to avoid accidental variable capture without having to go to the bother of doing it properly using gensym - the expectation being that you're unlikely to export a function called _pkgTable or _func etc.

Andrew

hmm, unfortunately the SKILL debugger doesn't work well with macros so I try to stay away from them.  I guess it doesn't take into account the macro when identifying the line number of an error.  So when it catches an error it will often highlight the wrong line and pretend to stop at a line that makes no sense.  Have you seen this problem before also?

I use the SKILL IDE and debugger.  When an error is caught and there is a macro around the error's code, the debugger will not point to the correct line in the code.  This makes it harder to debug.  I used to use a different test framework that used macros and suffered from this problem.

I've not seen that, and I just experimented with using the code above, even putting it inside a function so that the effects of lazy compilation might show up, and putting additional code beyond the macro call, with breakpoints on those lines and all worked fine.

Macros should be used sparingly because you're effectively creating a new language when you use them and that can be jarring for a reader of the code, but this seems a pretty good application for macros. You could write a function instead and use run-time evaluation - which in this case since it's only going to be at load time wouldn't be too awful - but it's much less elegant (and I see no good reason given that the macro works fine with the IDE). I'm not aware of issues with the IDE with macros - maybe they have been resolved?

(defun PkgExportFunc (env @rest functions)
  (let ((pkgTable (makeTable 'pkgTable nil)))
    (foreach func functions
	     (setarray pkgTable func (symeval func env)))
    pkgTable))

and then use:

(PkgExportFunc (theEnvironment) 'set 'get)

to do the export.

Andrew

Curtis,

On reflection (and I’ve. It tried this), the one place I can imagine the IDE might struggle with is putting  break points on code within the arguments of a macro because it doesn’t know which line that ends up on. Possibly… I’d have to check. Obviously that’s not going to be something you’d want to do with the macro I suggested as the arguments are symbols and not code.

So i don’t see this as a reason to avoid all macros.

Andrew

Awesome! That looks great, thanks Andrew Beckett!