search

BIDMCDigitalPsychiatry / LAMP-platform

The LAMP Platform (issues and documentation).
https://docs.lamp.digital/
Other
12 stars 10 forks source link

Inconsistent JSONata Activity behavior #665

Closed lukeoftheshire closed 1 year ago

lukeoftheshire commented 2 years ago

As seen in https://docs.lamp.digital/data_science/jsonata , $LAMP.Activity.list() by default uses true for the ignore_binary param. This behavior is different from other API implementations (to my mind both the JS and Python versions of the Activity list(/all_by_participant) function have ignore_binary set to false by default.

This behavior should be normalized across different languages to minimize difficulty when swapping between them - I would suggest adapting the JSONata command to fit the JS and Python versions as those are more commonly used and are the 'default' behavior.

avaidyam commented 2 years ago

@lukeoftheshire Are you suggesting we just change the default value of the parameter from true to false in LAMP-server? Or do you mean we need to update all the packages (LAMP-js, LAMP-py, LAMP-r, etc.)?

mcurbelo-orangeloops commented 1 year ago

I've just created a PR for this. It looks like Activity.Sensor.list() also has the default set to true (here Should I also change that one to false so it is consistent across the board ?

avaidyam commented 1 year ago

Sure! That would be helpful. Also, jsonata upstream has finally merged my Promise-based rewrite so it should be possible to check whether we can switch to using the main library instead? It's not urgent by any means though.

avaidyam commented 1 year ago

@carlan1 @ertjlane Please test this one out!

@Linoy339 @sarithapillai8 Please confirm that this change works for your team.

Linoy339 commented 1 year ago

@avaidyam . Will check and let you know asap