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

Fix param exports #579

Merged
merged 3 commits into from
Oct 17, 2019
Merged

Fix param exports #579

merged 3 commits into from
Oct 17, 2019

Conversation

rjplevin
Copy link
Collaborator

Stopped exporting connected params, and added a call to the new function propagate_params!() to the code emitted by @defcomposite.

…composites (not just ModelDefs)

- Removed function import_params!
- Added function propagate_params!
- Added call to propagate_params! to code emitted by @defcomposite, now exporting only unconnected params.
- Deprecated old function name() in favor of nameof().
Copy link
Collaborator

@corakingdon corakingdon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I trust you tried this out with the reverted version of FUND you created? (fund-model/MimiFUND.jl#53)

Since the dependency tests that are running here are still using the version where the population component's name was changed right?

@lrennels lrennels closed this Oct 17, 2019
@lrennels lrennels reopened this Oct 17, 2019
@codecov-io
Copy link

codecov-io commented Oct 17, 2019

Codecov Report

Merging #579 into master will increase coverage by 0.09%.
The diff coverage is 96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #579      +/-   ##
==========================================
+ Coverage   77.39%   77.49%   +0.09%     
==========================================
  Files          39       39              
  Lines        2584     2573      -11     
==========================================
- Hits         2000     1994       -6     
+ Misses        584      579       -5
Impacted Files Coverage Δ
src/core/types/defs.jl 79.31% <ø> (+1.34%) ⬆️
src/core/build.jl 93.26% <ø> (-0.07%) ⬇️
src/core/defs.jl 79.19% <100%> (-1.05%) ⬇️
src/core/defcomposite.jl 77.27% <100%> (ø) ⬆️
src/core/connections.jl 89.04% <95.45%> (+0.2%) ⬆️
src/mcs/mcs_types.jl 80.85% <0%> (+1.68%) ⬆️
src/core/paths.jl 88.23% <0%> (+5.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96014df...30a1bd0. Read the comment docs.

@lrennels
Copy link
Collaborator

@rjplevin shall I merge this?

@rjplevin
Copy link
Collaborator Author

Sure, once the checks complete...

@lrennels lrennels merged commit 5143400 into master Oct 17, 2019
@lrennels lrennels deleted the fix-param-exports branch October 17, 2019 22:31
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.

4 participants