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

remove composition.py #192

Merged
merged 8 commits into from
May 23, 2022
Merged

remove composition.py #192

merged 8 commits into from
May 23, 2022

Conversation

eagmon
Copy link
Member

@eagmon eagmon commented May 18, 2022

This removes vivarium-core dependence from composition.py, and adds a deprecation warning. This file holds a collection of helper functions for bringing processes together. In the last year composite.merge and Engine have been made much easier to use directly instead of relying on helper functions. Because of this composition.py is no longer helpful and in some cases made upkeep harder.

directories.py now has a few standard directories -- PROCESS_OUT_DIR, COMPOSITE_OUT_DIR, etc.

This will be a breaking change and I expect many dependent libraries will have to update their code to remove the helper functions. Some tips for this:

Instead of simulate_process:

    my_composite = my_process.generate()
    sim = Engine(composite=my_composite)
    sim.update(total_time)
    timeseries = sim.emitter.get_timeseries()

instead of simulate_composer:

    sim = Engine(
        composite=composite,
        initial_state=initial_state
        )
    sim.update(time_total)
    output = sim.emitter.get_timeseries()

adding a timeseries process:

    from vivarium.processes.timeline import TimelineProcess
    # create the processes
    agent = ToyAgent({'agent_id': agent_id})
    timeline_process = TimelineProcess({'timeline': timeline})

    # compose
    composite = agent.generate(path=('agents', agent_id))
    composite.merge(
        processes={'timeline': timeline_process},
        topology={'timeline': {
            'global': ('global',),
            'agents': ('agents',)}},
    )

By creating this pull request, I agree to the Contributor License
Agreement, which is available in CLA.md at the top level of this
repository.

… and can be handled directly instead of relying on helper functions.
@eagmon eagmon requested a review from a team as a code owner May 18, 2022 16:01
@U8NWXD
Copy link
Member

U8NWXD commented May 19, 2022

Since this is a breaking change, should it go in a 2.0 release? I think there are other breaking changes we've been wanting to make--should those all go in together?

@eagmon
Copy link
Member Author

eagmon commented May 19, 2022

@U8NWXD -- This could go into a 2.0 release, but it alone would not warrant a 2.0 release. What is the best practice for working on a number of changes and then releasing them all at once as a new version? Maybe this should go into a dev branch?

@U8NWXD
Copy link
Member

U8NWXD commented May 21, 2022

I looked into this and proposed a workflow in #194 for developing v2 while still maintaining v1

@eagmon
Copy link
Member Author

eagmon commented May 23, 2022

@U8NWXD -- I'm in support of your proposal for v2. But to put that off for a bit I brought composition.py back in this PR with a deprecation warning. So the main change for this PR is that Vivarium-core no longer uses composition.py, even though it is still available for dependent libraries. This will make it easier to remove when the time comes.

@eagmon eagmon merged commit d4d5168 into master May 23, 2022
@eagmon eagmon deleted the remove-composition branch May 23, 2022 20:14
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.

None yet

2 participants