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

Develop macro refactoring #84

Merged
merged 13 commits into from
May 12, 2020
Merged

Develop macro refactoring #84

merged 13 commits into from
May 12, 2020

Conversation

bvdmitri
Copy link
Member

@bvdmitri bvdmitri commented May 4, 2020

This pull request is a part of #82 issue:

Pull request covers some macro source-code, fixes anti-patterns and is aimed at improving macro code readability and robustness.

  1. @RV:
    • macro definition has been split into several functions with different logic
    • macro definition and tests have been moved from probability_distribution.jl to variable.jl
    • tests added for unsupported usage of @RV macro
  2. @symmetrical:
    • macro definition has been reimplemented from scratch
    • evals have been removed
    • support for multiple where blocks
    • fixes out-of-scope error while using this macro outside ForneyLab main module (e.g. experimental nodes which are defined in other modules)
    • tests added
  3. *Rule macros:
    • strings manipulations have been replaced with Julia expressions
  4. @ensureVariables:
    • strings manipulations have been replaced with Julia expressions

Copy link
Collaborator

@ThijsvdLaar ThijsvdLaar left a comment

Choose a reason for hiding this comment

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

Thanks a lot, I love it, especially the map and reduce in the rule validation.

@ivan-bocharov
Copy link
Collaborator

Thanks for the PR @bvdmitri. I think it can be merged into master.

@ThijsvdLaar ThijsvdLaar merged commit 49e094f into master May 12, 2020
@ThijsvdLaar ThijsvdLaar deleted the develop-macro-refactoring branch May 12, 2020 14:15
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.

3 participants