-
-
Notifications
You must be signed in to change notification settings - Fork 74
Model formatting #539
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
Model formatting #539
Changes from all commits
e598918
27a219f
c98a625
0ab1410
0ff40a7
6c9c762
fc2d638
e1bd06a
26e86d5
e5d7531
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,10 +9,11 @@ | |
import sys | ||
from collections import OrderedDict | ||
from concurrent.futures import ThreadPoolExecutor | ||
from datetime import datetime | ||
from io import StringIO | ||
from multiprocessing import cpu_count | ||
from pathlib import Path | ||
from typing import Any, Callable, Dict, List, Mapping, Optional, Union | ||
from typing import Any, Callable, Dict, Iterable, List, Mapping, Optional, Union | ||
|
||
import ujson as json | ||
from tqdm.auto import tqdm | ||
|
@@ -39,6 +40,7 @@ | |
MaybeDictToFilePath, | ||
SanitizedOrTmpFilePath, | ||
cmdstan_path, | ||
cmdstan_version, | ||
cmdstan_version_before, | ||
do_command, | ||
get_logger, | ||
|
@@ -297,6 +299,98 @@ def src_info(self) -> Dict[str, Any]: | |
get_logger().debug(e) | ||
return result | ||
|
||
def format( | ||
self, | ||
overwrite_file: bool = False, | ||
canonicalize: Union[bool, str, Iterable[str]] = False, | ||
max_line_length: int = 78, | ||
*, | ||
backup: bool = True, | ||
) -> None: | ||
""" | ||
Run stanc's auto-formatter on the model code. Either saves directly | ||
back to the file or prints for inspection | ||
|
||
|
||
:param overwrite_file: If True, save the updated code to disk, rather | ||
than printing it. By default False | ||
:param canonicalize: Whether or not the compiler should 'canonicalize' | ||
the Stan model, removing things like deprecated syntax. Default is | ||
False. If True, all canonicalizations are run. If it is a list of | ||
strings, those options are passed to stanc (new in Stan 2.29) | ||
:param max_line_length: Set the wrapping point for the formatter. The | ||
default value is 78, which wraps most lines by the 80th character. | ||
:param backup: If True, create a stanfile.bak backup before | ||
writing to the file. Only disable this if you're sure you have other | ||
copies of the file or are using a version control system like Git. | ||
""" | ||
if self.stan_file is None or not os.path.isfile(self.stan_file): | ||
raise ValueError("No Stan file found for this module") | ||
try: | ||
cmd = ( | ||
[os.path.join(cmdstan_path(), 'bin', 'stanc' + EXTENSION)] | ||
# handle include-paths, allow-undefined etc | ||
+ self._compiler_options.compose_stanc() | ||
+ [self.stan_file] | ||
) | ||
|
||
if canonicalize: | ||
if cmdstan_version_before(2, 29): | ||
if isinstance(canonicalize, bool): | ||
cmd.append('--print-canonical') | ||
else: | ||
raise ValueError( | ||
"Invalid arguments passed for current CmdStan" | ||
+ " version({})\n".format( | ||
cmdstan_version() or "Unknown" | ||
) | ||
+ "--canonicalize requires 2.29 or higher" | ||
) | ||
else: | ||
if isinstance(canonicalize, str): | ||
cmd.append('--canonicalize=' + canonicalize) | ||
elif isinstance(canonicalize, Iterable): | ||
cmd.append('--canonicalize=' + ','.join(canonicalize)) | ||
else: | ||
cmd.append('--print-canonical') | ||
|
||
# before 2.29, having both --print-canonical | ||
# and --auto-format printed twice | ||
if not (cmdstan_version_before(2, 29) and canonicalize): | ||
cmd.append('--auto-format') | ||
|
||
if not cmdstan_version_before(2, 29): | ||
cmd.append(f'--max-line-length={max_line_length}') | ||
elif max_line_length != 78: | ||
raise ValueError( | ||
"Invalid arguments passed for current CmdStan version" | ||
+ " ({})\n".format(cmdstan_version() or "Unknown") | ||
+ "--max-line-length requires 2.29 or higher" | ||
) | ||
|
||
out = subprocess.run( | ||
cmd, capture_output=True, text=True, check=True | ||
) | ||
if out.stderr: | ||
get_logger().warning(out.stderr) | ||
result = out.stdout | ||
if overwrite_file: | ||
if result: | ||
if backup: | ||
shutil.copyfile( | ||
self.stan_file, | ||
self.stan_file | ||
+ '.bak-' | ||
+ datetime.now().strftime("%Y%m%d%H%M%S"), | ||
) | ||
with (open(self.stan_file, 'w')) as file_handle: | ||
file_handle.write(result) | ||
else: | ||
print(result) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Printing is not very pythonic. Should it just return the result? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am thinking of how a user would use this method and I have a hard time imagining when they'd actually want the string as a result. If anything it encourages the style of programming we don't want, where the model code ends up stored as a string in the python code. The workflow I'm imagining for this function is that it will either be used so that someone can preview the output, in which case printing is the right thing to do, or they will be batch-processing stan files and formatting them, in which case they'd be saving them back to disk. I can be persuaded that this is too narrow, however There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I think printing is ok in this case |
||
except (ValueError, RuntimeError) as e: | ||
raise RuntimeError("Stanc formatting failed") from e | ||
|
||
@property | ||
def stanc_options(self) -> Dict[str, Union[bool, int, str]]: | ||
"""Options to stanc compilers.""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,4 +6,6 @@ | |
# and we re-ignore hpp and exe files | ||
*.hpp | ||
*.exe | ||
!return_one.hpp | ||
*.testbak | ||
*.bak-* | ||
!return_one.hpp |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
generated quantities { | ||
array[10,10,10,10,10] matrix[100,100] a_very_long_name; | ||
int x = (((10))); | ||
int y; | ||
if (1) | ||
y = 1; | ||
else | ||
y=2; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# pound-sign comment | ||
generated quantities { | ||
int x; | ||
x <- (((((3))))); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.