Skip to content

Allow #include statements to function in stanc.js#1432

Closed
WardBrian wants to merge 8 commits intomasterfrom
feature/stancjs-include-paths
Closed

Allow #include statements to function in stanc.js#1432
WardBrian wants to merge 8 commits intomasterfrom
feature/stancjs-include-paths

Conversation

@WardBrian
Copy link
Copy Markdown
Member

@WardBrian WardBrian commented Jun 21, 2024

Closes #1430

This PR can be viewed as several parts:

  1. Some boilerplate-y code in stancjs.ml to read a { [s: string] : string } JS object into a ocaml string String.Map.t
  2. Extracting the logic in the Preprocessor that looks up new included files into a module type that is passed in from the outside
  3. Doing the same in the Lexer

An earlier version of this didn't use functors and just had Preprocessor store a function pointer that was set by stancjs.ml/stanc.ml, but I had both aesthetic and performance concerns with that approach. These functors are ultimately pretty lightweight

This is currently lightly tested in stanc.js but I intend to add more.

Submission Checklist

  • Run unit tests
  • Documentation
    • If a user-facing facing change was made, the documentation PR is here:
    • OR, no user-facing changes were made

Release notes

stanc.js can now accept models which contain #include statements. A fourth argument is available on the javascript stanc function which must be either undefined or a object mapping included file names to Stan source code as strings.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

@WardBrian WardBrian requested a review from nhuurre June 21, 2024 15:29
@WardBrian WardBrian linked an issue Jun 21, 2024 that may be closed by this pull request
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 10 lines in your changes missing coverage. Please review.

Project coverage is 89.75%. Comparing base (b46cc7e) to head (03115e1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1432      +/-   ##
==========================================
- Coverage   89.83%   89.75%   -0.08%     
==========================================
  Files          63       65       +2     
  Lines       10484    10495      +11     
==========================================
+ Hits         9418     9420       +2     
- Misses       1066     1075       +9     
Files Coverage Δ
src/frontend/Filesystem_includes.ml 100.00% <100.00%> (ø)
src/frontend/Parse.ml 81.57% <100.00%> (+1.57%) ⬆️
src/frontend/Preprocessor.ml 96.22% <100.00%> (-0.70%) ⬇️
src/stanc/stanc.ml 80.71% <100.00%> (+0.13%) ⬆️
src/frontend/Pretty_print_prog.ml 82.35% <80.00%> (-0.51%) ⬇️
src/frontend/In_memory_includes.ml 0.00% <0.00%> (ø)

@WardBrian
Copy link
Copy Markdown
Member Author

Superseded by #1433

@WardBrian WardBrian closed this Jun 24, 2024
@WardBrian WardBrian deleted the feature/stancjs-include-paths branch July 9, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

#include support in stancjs

1 participant