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

Make warnings more uniform #253

Open
Leo-Send opened this issue Mar 5, 2024 · 1 comment
Open

Make warnings more uniform #253

Leo-Send opened this issue Mar 5, 2024 · 1 comment
Assignees
Milestone

Comments

@Leo-Send
Copy link
Contributor

Leo-Send commented Mar 5, 2024

Description

PR#248 included a warning using the 'warning()' function which behaves differently from the 'logging::logwarn' function used elsewhere in the project. When using 'warning()', warnings are enumerated and repeated at the end of testruns and normal execution. When using 'logwarn()', the warning just appears in the logs alongside 'loginfo()' (albeit recoloured). Also, 'warning()' is used by external libraries used in coronet.

Motivation

It would be nice to make warnings uniform or introduce a distinction between the two types, so that you can more easily interpret the warning and act accordingly.
I would prefer having specific cases when to use which warning, as the 'warning()' function produces warnings that are much harder to overlook whereas 'logwarn()' warnings can get lost in the hundreds of lines of log messages this project can produce. Thus it would make sense to use 'warning()' warnings in cases where the functionality of the program is likely to be broken, while using 'logwarn()' warnings in cases where the execution can most likely proceed normally, though some information might be lost on the way.

Ideas for the Implementation

Either only use one of the types or assign specific use cases for each of the warning types.

Additional Information

The new warning can be seen in 'util-networks-misc.R' line 154 and occurs during a test run of 'tests/test-util-networks-misc.R'.

@Leo-Send
Copy link
Contributor Author

Leo-Send commented Mar 8, 2024

These are the warnings I found that i believe might profit from using 'warning()':

  • every warning that mentions cleanup functions in util-data --- if execution continues without using cleanup: might lead to crash
  • util-networks-covariates line 1620 (get.first.activity.data) --- returns na, might lead to crash
  • util-networks line 1880 (get.data.sources.from.relations) --- returns na, might lead to crash
  • util-tensor line 157 (get.author.networks.for.multiple.relations)--- returns na, might lead to crash

There is also a warning that i believe is not helpful at all to a user:

  • util-data-misc line 550 (get.mail.thread.originating.mailing.list)

@bockthom bockthom added this to the Future milestone Mar 22, 2024
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

2 participants