search

JGCRI / hector

The Hector Simple Climate Model
http://jgcri.github.io/hector/
GNU General Public License v3.0
107 stars 45 forks source link

fix : convert trailing comments to single sline comments #711

Closed gaelforget closed 9 months ago

gaelforget commented 11 months ago

Seems safer since fewer parsers will support comments trailing section lines than single line comments.

With at least one parser, IniFile.jl, these comments lead to an incorrect read -- separating out the comments as single lines fixed it.

bpbond commented 11 months ago

Thanks @gaelforget . What's the use case for this? I.e., why would another parser be reading Hector's INI files?

codecov-commenter commented 11 months ago

Codecov Report

Merging #711 (32d85eb) into master (8087ca7) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 32d85eb differs from pull request most recent head d00c381. Consider uploading reports for the commit d00c381 to get more accurate results

@@           Coverage Diff           @@
##           master     #711   +/-   ##
=======================================
  Coverage   79.72%   79.72%           
=======================================
  Files          61       61           
  Lines        5978     5978           
=======================================
  Hits         4766     4766           
  Misses       1212     1212
gaelforget commented 11 months ago

Thanks @gaelforget . What's the use case for this? I.e., why would another parser be reading Hector's INI files?

Use case is running Hector via a Julia wrapper. I do this in the following notebook that is part of the example set for ClimateModels.jl (the generalized model wrapper).

http://www.gaelforget.net/notebooks/Hector.html

Similar to what's done in pyhector and the R interface I presume, we read in the ini file to display parameters, let user change parameters, save customized parameters to a new ini file, and call Hector from Julia.

kdorheim commented 10 months ago

@gaelforget, thanks for your interest in contributing to Hector! I guess I am curious as to why this change was only made for the HFC4310_halocarbon?

gaelforget commented 10 months ago

Other comments were single line comments or after variable definitions & did not seem to cause any issue -- unlike these few comments placed after section definitions ([HFC4310_halocarbon ]).

kdorheim commented 9 months ago

This does not affect our miscellaneous scripts that are not tested as part of the CI (this is a todo #657) good to merge!