bedatadriven / activityinfo-R

ActivityInfo R Language Client
https://www.activityinfo.org/support/docs/R/
17 stars 12 forks source link

Change copySchema() to copySchemaFromFields() or extractSchemaFromFields() #84

Closed nickdickinson closed 1 year ago

nickdickinson commented 1 year ago

I wonder if copySchema() function gives the false impression that one may be copying an existing schema from a form but in fact one is flattening the schema and simply copying the schema of each field in the remote records object. So that means one will possibly have a combination of reference fields and/or other types of fields from both the root form and other referenced forms.

I am thinking about this as I'm adding code to check for duplicate codes, etc. which are more likely to come about when one is flattening a schema. For now, I handle these codes by renaming with check.names and replacing . with _.

@Ryo-N7 @akbertram Are you ok with changing the name?

Ryo-N7 commented 1 year ago

ah yes, i didn't really think about this aspect. you make a good point

i was mostly focusing on testing out simple forms without really thinking about complicated sub-form or multiple sub-form structures and how that would affect the "copy" version of the schema

I think extractSchemaFromFields() makes more sense in how this function is implemented currently as we're extracting stuff from the "representation of the data" - the remote records object rather than copying the schema from the form/records/etc.

nickdickinson commented 1 year ago

Changed to extractSchemaFromFields()