Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create_variable() must include aliases for subreferences in payload #373

Closed
alextanski opened this issue Sep 12, 2019 · 4 comments
Closed
Assignees

Comments

@alextanski
Copy link

alextanski commented Sep 12, 2019

Currently, when creating a variable that is built from subvariables (i.e. categorical arrays and multiple responses), the payload only requires that their name attribute to be provided:

if subvariables:
    payload['subreferences'] = [
        {'name': item['name']} for item in subvariables
    ]

This results in the server enumerating the subvariables, appending -number to the parent variable name, e.g.:

for s in wave_ds['R3_Inmind'].items():
    s
(u'R3_Inmind-16', <Variable: name='Eriksson Bil'; id='0017'>)
(u'R3_Inmind-4', <Variable: name='Bilbolaget'; id='0005'>)
(u'R3_Inmind-27', <Variable: name='Imexa'; id='0028'>)
(u'R3_Inmind-14', <Variable: name='Borås'; id='0015'>)
(u'R3_Inmind-6', <Variable: name='Bilia'; id='0007'>)
[...]

We are already passing id and alias for each of the items into the method, so it can be corrected by simply also getting those properties from the input if provided:

if subvariables:
    payload['subreferences'] = [
        {'name': item['name'],
         'alias': item['alias'],
         'id': item['id']
         } for item in subvariables
    ]

The current behavior is problematic because of two things:

  1. It is not consistent with the way the "reversed" way of the conversion from Crunch to QP is working, for example in Crex and general SSC DP.

  2. It is leading to metadata conflicts in tracker scenarios, because the way DP has set up the dataset pre-Crunch is not reflected when a variable is created based on the definition they are seeing in their QP file.

The problem cannot be worked around (especially for MR variables), as we are treating multiple responses questions as "normal" questions, not as an array, so DP is not able to control the subvariable names manually by renaming. They are part of the metadata conversion.

tagging @jamesrkg.

@mathiasbc
Copy link
Contributor

@alextanski #374

@jamesrkg
Copy link

Thanks @mathiasbc !

@alextanski
Copy link
Author

thanks @mathiasbc, looks good. We can, however, leave out the code for getting the id from the subvariable definition, as (of course) Crunch generates that one directly from the url created for the subreference, hence no point to pass/get() it.

Thanks for the quick fix! 👍

@xbito
Copy link
Contributor

xbito commented Sep 15, 2019

The pull request has been merged after the latest fix from Mathias. Closing.

@xbito xbito closed this as completed Sep 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants