@RahilJain1366 Thanks for this work.

I finally looked at it, and it seemed like some of the things like the NumbersReal etc. were unnecessary. While working out the alternatives, one thing led to another, and the resulting set of changes was large enough that it was easier to just do them than to try to explain what needed to be done next. I took the liberty of pushing the changes directly to your branch; I hope you don't mind – it just seemed faster all around. Hopefully the code diffs will be useful to understand the pattern of changes.

@dstrain115 I think someone other than me should review the final version of this PR; would you be willing?

Hi @mhucka, thanks for taking the time to review and push the updates! I should have kept it as Real instead of naming NumbersReal, it looks cleaner.

Was there a reason behind doing this change? While numbers.Real is more general, isinstance checks for numbers.Real is slower and it can sometimes cause more problems for callers of the function, since they need to deal with a more general return type.

Since this is in ReCirq, it's probably fine, since this code is not likely used outside the library itself.

I am just wondering as to the motivation for this.

Was there a reason behind doing this change? While numbers.Real is more general, isinstance checks for numbers.Real is slower and it can sometimes cause more problems for callers of the function, since they need to deal with a more general return type.

A very excellent question!

This PR is in response to issue #435, which itself came in response to discussions on PR #430. The latter PR addressed some flake8 warnings about type(…) == X constructs that should be updated to isinstance. In a comment there, Pavol noted that places where isinstance(foo, float) was being used might be better done as isinstance(foo, numbers.Real) in order to allow for values that are either Python float or Numpy floats. I then opened issue #435 to remind us to do something about that.

That's a good point about the implication of return types. Should this PR be changed to avoid returning numbers.Real?

Yeah, I think I would be more comfortable with removing the return type changes. I think it's better practice if we change inputs to be as general as possible but keep outputs specific as possible. This makes the functions most flexible to use.

Otherwise, (soft) LGTM