dollabs / pamela

Probabalistic Advanced Modeling and Execution Learning Architecture
Apache License 2.0
233 stars 13 forks source link

Issue 136 #140

Closed tmarble closed 7 years ago

tmarble commented 7 years ago

Please see the commit message and additional comments in #136

pmdoll commented 7 years ago

untitled

tmarble commented 7 years ago

@pmdoll

  1. Use of symbols for fields is consistent everywhere
  2. A field-ref is a specialization of symbol-ref where 'this is elided from the name vector because fields references always start from this class.
  3. Yes. One may refer to a method of pclass initialized in a field like field1.methodAor the field of another pclass like field2.field3
pmdoll commented 7 years ago

syntax error is reported as grammar is ambigous.

(defpclass a []
  :fields {:breathing true})
tmarble commented 7 years ago

@dcerys i believe the regressions you have found have been addressed in https://github.com/tmarble/pamela/commit/e6469721385450e9f1ed3bbb5d08016cd8d356fa

tmarble commented 7 years ago

@dcerys I have just pushed an additional commit to address the issues from yesterday's telecon (please see the commit message).

pmdoll commented 7 years ago

Removal of plantid will have ripple effects to other components.

tmarble commented 7 years ago

@pmdoll do you want to keep :plantid in addition to OR in replacement of :plant-id ?

dcerys commented 7 years ago

It is good to inch forward on the name harmonization. Since we agreed to do this in a backwards-compatible way, we should preserve the old attributes while adding the new ones. This will result in some temporary redundancy, but will maintain short-term compatibility.

So, we should do this for :plant-id, :plant-interface, and :plant-part. All new/updated code should be using these new attributes.

pmdoll commented 7 years ago

Since this is a trivial change, I suggest we leave it as is.

tmarble commented 7 years ago

@dcerys I'm not super clear from your previous comments what needs to change in order for the PR to be merged? The formal args for the video function are: [client server display] and we know that argsmap is NOT guaranteed to be in positional argument order (hence our new design for args-map). Please advise.

tmarble commented 7 years ago

@dcerys please review this latest commit.