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

add lang_header arg for specify language header values #215

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jdugh
Copy link

@jdugh jdugh commented Apr 29, 2020

add lang_header arg in export def. lang_header can be used for specify the language for header values in excel/csv export (you can have _xml header values and _default values for submits datas). If not set, lang arg override lang_header arg value.

…y the language for header values in excel/csv export (you can have _xml header values and _default values for submits datas). If not set, lng arg override lang_header value.
@coveralls
Copy link

coveralls commented Apr 29, 2020

Coverage Status

Coverage increased (+0.007%) to 83.958% when pulling 4ee322d on jdugh:lang_header into 8628d7e on kobotoolbox:master.

jdugh added a commit to jdugh/kpi that referenced this pull request Apr 29, 2020
…nguage for header values in excel/csv export (you can have _xml header values and _default values for submits datas). If not set, lang arg override lang_header arg value. Need this PR : kobotoolbox/formpack#215
@dorey
Copy link
Contributor

dorey commented Apr 29, 2020

Hi @jdugh . Thanks for the PR. Could you add a test that uses this column and shows how the values would change when this parameter is used?

Also, if I understand correctly what this does, it might make more sense to call it header_lang or header_language.

@@ -337,7 +337,7 @@ def to_json(self, **kwargs):
def export(self, lang=UNSPECIFIED_TRANSLATION, group_sep='/', hierarchy_in_labels=False,
versions=-1, multiple_select="both",
force_index=False, copy_fields=(), title=None,
tag_cols_for_header=None):
tag_cols_for_header=None, lang_header=-1):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tag_cols_for_header=None, lang_header=-1):
header_lang=None,
tag_cols_for_header=None):

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we cannot set None to default value for "header_lang" otherwise we cannot know if it is not set or set to None. And, if user call with : (lang=False,header_lang=None) I don't want to change value of arg header_lang. If user call with (lang=False) - header_lang is not set, I want to override header_lang with lang value for not change the current algo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. Thanks for reminding me of this. I will discuss this with the team and see if we can agree on -1 or a different solution.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @dorey , do you know if you can agree a -1 value (or a different solution), and also, accept this PR in the futur ?

@jdugh
Copy link
Author

jdugh commented Apr 30, 2020

Hi @jdugh . Thanks for the PR. Could you add a test that uses this column and shows how the values would change when this parameter is used?

Also, if I understand correctly what this does, it might make more sense to call it header_lang or header_language.

Ok thank you. I go to create the test and change the name to header_lang. Here and in KPI views.

@jdugh
Copy link
Author

jdugh commented Aug 11, 2021

@joshuaberetta I have merge my PR with the master and I have change the -1 default value with a constant.

@jdugh
Copy link
Author

jdugh commented Jan 30, 2023

Hi @joshuaberetta and @dorey,
Do you think this PR can be merge with master in futur ?

@jdugh
Copy link
Author

jdugh commented Feb 6, 2024

Hi @jnm.
Do you have any feedback about this one.
(This is a little up 😅 - PR without merge conflicts and with some unit test 😅)

EDIT :
The main objective of this feature is to allow to specify the language for header values in excel/csv/... export :
the 'name' question in header, and the 'label' response (mainly for choices list) in the data.
Currently you can only have full data (header and data) with 'name' or with 'label' through lang key in export API.
My end goal is to be able to download the data through API, with this structure.

Otherwise, Unless I'm mistaken, the endpoint on KPI API : /api/v2/assets/<ID_STRING>/data/?format=json does not offer to recover the 'question name' in json key, and the 'label' in values. If we could imagine the same system on this endpoint, that would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants