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

include macros functions in rule._function #451

Merged
merged 3 commits into from Sep 9, 2019

Conversation

@ortega2247
Copy link
Contributor

ortega2247 commented Jul 10, 2019

Changed the code to include macros functions in the _function attribute of the Component class. This could be helpful as a way to annotate rules with the macros that generate them.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 10, 2019

Coverage Status

Coverage increased (+0.04%) to 78.621% when pulling f367043 on LoLab-VU:rule_function into 4f9f4a1 on pysb:master.

Copy link
Member

alubbock left a comment

Since _function is a private API, I guess this can be changed. but we need to decide what the behaviour should be. With this change, it uses the innermost function as the name, even if that's a macro (see case 4, below). I wrote some unit tests to check this, which you could add to your PR in test_core.py, for example.

@with_model
def test_function_introspection():
    # Case 1: Component defined inside function
    Monomer('A')
    assert A._function == 'test_function_introspection'

    # Case 2: Component defined inside nested function
    def define_monomer_b():
        Monomer('B')
    define_monomer_b()
    assert B._function == 'define_monomer_b'

    # Case 3: Component defined by macro
    from pysb.macros import equilibrate
    equilibrate(A(), B(), [1, 1])

    print(model.rules)
    assert model.rules['equilibrate_A_to_B']._function == 'equilibrate'

    # Case 4: Component defined by macro inside function
    def define_macro_inside_function():
        Monomer('C')
        equilibrate(A(), C(), [2, 2])
    define_macro_inside_function()
    assert model.rules['equilibrate_A_to_C']._function == 'equilibrate'
pysb/core.py Outdated Show resolved Hide resolved
@jmuhlich

This comment has been minimized.

Copy link
Member

jmuhlich commented Jul 12, 2019

This makes me think this feature should be under more user control, as some might want macros included, some might not, and some might indeed want the whole stack. We shouldn't try to make the existing code any hairier than it already is, but in a future version maybe we can work out a way to reengineer this feature "orthogonally" to the Component class itself.

@alubbock

This comment has been minimized.

Copy link
Member

alubbock commented Jul 12, 2019

Agreed @jmuhlich, probably best to capture the whole function stack in a future version for maximum flexibility. Not sure how it could be done outside of the class itself - maybe some kind of hook mechanism?

For now, this LGTM. Thanks @ortega2247.

@alubbock alubbock merged commit f6ee586 into pysb:master Sep 9, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@alubbock alubbock deleted the LoLab-VU:rule_function branch Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.