Epiconcept-Paris / STRAP-epiuf

Utility function package for STRAP
0 stars 0 forks source link

ApplyDictionnary warning #90

Closed epi-gde closed 11 months ago

epi-gde commented 1 year ago

ApplyDictionnary could check content of xlsx file to issue warning if information are missing or incorrect

loremerdrignac commented 1 year ago

@epi-gde to provide an example

epi-clz commented 1 year ago

Hi @loremerdrignac and @epi-gde , As agreed, I've added some improvements to openDictionary. There are 2 issues the user can face by loading a dictionary: 1) The dictionary has different sheet names 2) The dictionary has a blank dictionary sheet (no column names and no content) Thus, I've added a warning message that will be printed on the console and will stop the process.

I thought this was at the applyDictionary stage but indeed, this happens a bit before when calling openDictionary(). I have created a branch called applyDictionary_2023_09_18 - should be names as openDictionary_2023_09_18, apologies. I have commented all the improvements with: ## PR_CLZ ## END_PR_CLZ I know it's not a PR but... I didn't know how to call it : )

Next steps suggestions:

loremerdrignac commented 1 year ago

Add a message if we are missing any of the essential columns (saying which columns are missing so that the user is warned that R has automatically created a blank column)

epi-clz commented 1 year ago

Fixed @loremerdrignac @epi-gde

One suggestion: Do you think we are returning a lot of messages by using the dictionary function? E.g., the message "the columns blabla,blabla were added as blank" are lost on the whole set. On my side, when I run the dictionaries I do not tun only one but several and this will start filling the console with messages. I would find a way to summarize them, what do you think? I suggest: "3 sheets: dictionary, dicos, actions loaded. X columns added as blank." And that's it

strap
loremerdrignac commented 1 year ago

Dear @epi-clz Thank you for this!! Unfortunately, I am not a *Dictionnary user :) but you manage to put the same information within less words, I think that's great! Should we quickly ask the STRAP team on the chat maybe? Thanks again!

epi-clz commented 1 year ago

Thanks! I've added what Gilles suggestion: blank line before and after the red messages when importing dictionary @epi-gde @loremerdrignac

epi-clz commented 1 year ago

Hi! I added the warning with the sheet name that is missing in the dictionary. Thanks for the suggestion! @epi-gde @loremerdrignac @epi-mml @epi-jhd