EranOfek / AstroPack

Astronomy & Astrophysics Software Pacakge
Other
17 stars 4 forks source link

`warning()` and `error()` in the code #9

Open EastEriq opened 3 years ago

EastEriq commented 3 years ago

IMHO at least warnings should be treated by the logger, not natively by matlab. Like in this line

https://github.com/EranOfek/AstroPack/blob/6a77aec397ce593cd2e005d8d301395e2cbd7c3b/matlab/external/%2Byaml/ReadYamlRaw.m#L66

But maybe there is a design choice a priori which I don't understand. I noted for example the nsfe persistent flag.

Btw:

AstroPack/matlab$ grep -r warning\(| wc -l
97

AstroPack/matlab$ grep -r error\(| wc -l
1898
EastEriq commented 3 years ago

Ditto, or even worse, for fprintf(). those which actually write to a file are ok [as well as those writing to a serial or tcp device], but for writing to stdout, I strongly doubt.

AstroPack/matlab$ grep -r fprintf | wc -l
660

(not differentiating the two uses)

EranOfek commented 3 years ago

I don't understand the problem - why not using fprintf when writing to stdout? eran

On Wed, Jun 16, 2021 at 4:23 PM EastEriq @.***> wrote:

Ditto, or even worse, for fprintf(). those which actually write to a file are ok [as well as those writing to a serial or tcp device], but for writing to stdout, I strongly doubt.

AstroPack/matlab$ grep -r fprintf | wc -l 660

(not differentiating the two uses)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/EranOfek/AstroPack/issues/9#issuecomment-862376186, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJUQ4MWZW74YKV75C6TGTDTTCQUDANCNFSM46ZLHEFA .

EastEriq commented 3 years ago

For two reasons.

  1. it can't be squelched by a general verbosity setting
  2. it is not logged in a consistent way An example of how odd fprintf's become obtrusive is https://github.com/EranOfek/AstroPack/issues/7#issuecomment-862377687
EranOfek commented 3 years ago

Chen, At some point we need to modify all error/warning/fprinf to call our logger. eran

On Thu, Jun 17, 2021 at 10:34 AM EastEriq @.***> wrote:

For two reasons.

  1. it can't be squelched by a general verbosity setting
  2. it is not logged in a consistent way An example of how odd fprintf's become obtrusive is #7 (comment) https://github.com/EranOfek/AstroPack/issues/7#issuecomment-862377687

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/EranOfek/AstroPack/issues/9#issuecomment-863004999, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJUQ4MS5ESPX6SZORO66QLTTGQR7ANCNFSM46ZLHEFA .

EastEriq commented 3 years ago
body p { margin-bottom: 0cm; margin-top: 0pt; } 

Is there some reason why Chen is not on
  the watchlist of the issues and communication depends on your time
  to forward messages?

On 17/06/2021 10:50, EranOfek wrote:

  Chen,
  At some point we need to modify all error/warning/fprinf to call
  our logger.
  eran

  On Thu, Jun 17, 2021 at 10:34 AM EastEriq ***@***.***> wrote:

  > For two reasons.
  >
  > 1. it can't be squelched by a general verbosity setting
  > 2. it is not logged in a consistent way
  > An example of how odd fprintf's become obtrusive is #7
  (comment)
  >

https://github.com/EranOfek/AstroPack/issues/7#issuecomment-862377687

— You are receiving this because you commented. Reply to this email directly, view it on GitHub

https://github.com/EranOfek/AstroPack/issues/9#issuecomment-863004999, or unsubscribe

https://github.com/notifications/unsubscribe-auth/ABJUQ4MS5ESPX6SZORO66QLTTGQR7ANCNFSM46ZLHEFA .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. [ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/EranOfek/AstroPack/issues/9#issuecomment-863014617", "url": "https://github.com/EranOfek/AstroPack/issues/9#issuecomment-863014617", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

EastEriq commented 3 years ago

Matlab warnings are a nuisance to me because I get traces like

P=obs.unitCS('2'); [INF] Adding property: obs.unitCS.2.create [INF] loadYaml: Loading file: /home/enrico/Eran/LAST/LAST_config/config/obs.unitCS/obs.unitCS.2.create.yml [ERR] loadYaml: File not found: /home/enrico/Eran/LAST/LAST_config/config/obs.unitCS/obs.unitCS.2.create.yml Warning: No such file to read: obs.unitCS.2.create.yml In yaml.ReadYamlRaw>load_yaml (line 66) In yaml.ReadYamlRaw (line 30) In yaml.ReadYaml (line 21) In Configuration.loadYaml (line 182) In Configuration/loadFile (line 91) In obs.LAST_Handle/loadConfig (line 14) In obs.unitCS (line 70)

The bold part is useless and distracting to me. Only the [ERR] line is of concern for the user.