Cleanup unnecessary @staticmethod annotations

[balopat]

Context: #3322 (comment)

[shouri007]

@balopat I'd like take this up. Should changes be made everywhere @staticmethod is used? Also, do you have any documentation with regards to setting up a development environment on Mac other than Debian?

[balopat]

Hi @shouri007 - thank you for taking this up, it's yours! Regarding mac we don't have the instructions but we do have a task to document it - I'm working on a mac myself - it is definitely possible. If you're interested taking that up as well, let me know - otherwise if you get stuck (I just use venv personally and pip install -r requirements.txt -r cirq/contrib/contrib-requirements.txt -r dev_tools/conf/pip-list-dev-tools.txt on python 3.8 + PyCharm, it should work pretty seamlessly) also let me know.

[balopat]

Should changes be made everywhere @staticmethod is used?

Yes, I believe so!

[thanacles]

Wouldn't this be a pretty big breaking change? For example, all calls to CircuitDag.make_node would need to be changed to make_node. Personally I like module functions more than static methods, but do you feel comfortable with going forward with this?

[thanacles]

Here's an option: we clean up all of the private functions that start with "_". That way we can feel safer that people using Cirq won't be broken. But if you feel comfortable just changing them all over, I can implement the refactor.

[daxfohl]

There is a @deprecated annotation cirq uses for depreciating things over time, to reduce breakages. Not sure what the policy is for private functions. It may be a judgment call, case by case.

[thanacles]

I'm willing to add @deprecated to all of the public static methods. Do you think I should? I could:

• Move the method out of the class
• Replace the method in the class with:
  @deprecated staticmethod(module_function)

That way the user could update their code to use the module function before we remove the static method completely

[balopat]

@deprecated should be the way to go for all public ones, for private ones we can just break them, people should not depend on private method.

[viathor]

We probably want to retain discoverability associated with having most names at the top level.

There are at least two ways of eliminating @staticmethod: make them @classmethod when appropriate, turn them into module-level functions. It's quite possible that a significant fraction of our @staticmethod actually qualify for the former treatment.

[thanacles]

The PR to refactor the private methods is ready (#4231). As far as @classmethod is concerned, check out this quote from the styleguide:

Use classmethod only when writing a named constructor or a class-specific routine that modifies necessary global state such as a process-wide cache.

It seems that since static methods don't "modif[y] necessary global state", the styleguide suggests that we should not refactor them into class methods.

I feel like the remaining work (if any) associated with this bug will require a pretty deep level of familiarity with the concerns of the remaining static methods. So I suggest we remove the "good first issue" tag.

Also, I will move on to other issues.

[balopat]

Thanks @thanacles - that's a fair assessment!

[dstrain115]

This was partially cleaned up a while ago. Remaining references are not going to get cleaned up due to backwards compatibility issues. Closing this issue.