Skip to content

Rename create_act_on_args to create_simulation_state #5299

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

Merged
merged 9 commits into from
Apr 30, 2022

Conversation

daxfohl
Copy link
Collaborator

@daxfohl daxfohl commented Apr 27, 2022

@95-martin-orion Here's this allows safely deprecating those functions. Plus ProductSimulationState.args is now ProductSimulationState.sim_states, and also renamed the args param for public act_on protocol.

@daxfohl daxfohl requested review from wcourtney, a team, vtomole, cduck and verult as code owners April 27, 2022 21:30
@daxfohl daxfohl requested a review from mpharrigan April 27, 2022 21:30
@CirqBot CirqBot added the size: L 250< lines changed <1000 label Apr 27, 2022
Copy link
Collaborator

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

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

Couple of issues. I'm also kind of blindly assuming there aren't "args" appearances outside of the changes here - how did you go about checking for these?

@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 29, 2022

There are definitely "args" appearances beyond these fixes. I don't have a good way of checking for all of them -- our other protocols like unitary also uses "args", not to mention it's a python keyword, so we can't just find/replace all.

However this PR does take care of all the regex 'act_on\w*_args' appearances at least, and I'm pretty sure it takes care of all appearances of "args" in public functions / properties. But there are still lots of places where local variables are just named "args", especially in tests. To completely kill them we probably have to skim through sim manually and replace whatever we find.

@95-martin-orion
Copy link
Collaborator

{...} this PR does take care of all the regex 'act_on\w*_args' appearances at least, and I'm pretty sure it takes care of all appearances of "args" in public functions / properties.

I think if we've hit these, that's enough to call this PR complete - we can catch the rest in code cleanup down the road.

@daxfohl daxfohl requested a review from 95-martin-orion April 29, 2022 23:10
@95-martin-orion 95-martin-orion merged commit 1612b87 into quantumlib:master Apr 30, 2022
@daxfohl daxfohl deleted the createacton branch April 30, 2022 12:53
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
* Change _create_act_on_args to _create_simulation_state

* Deprecate productsstate.args

* Test deprecate productsstate.args

* Test deprecate create_act_on_args

* Rename things within test

* Rename act_on arg, some tests

* lint

* Make deprecation warning text more explicit
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
* Change _create_act_on_args to _create_simulation_state

* Deprecate productsstate.args

* Test deprecate productsstate.args

* Test deprecate create_act_on_args

* Rename things within test

* Rename act_on arg, some tests

* lint

* Make deprecation warning text more explicit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: L 250< lines changed <1000
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants