Remove unused reverse argument in SmallestPerKey

[Ruwann]

Remove unused reverse argument in the SmallestPerKey PTransform as it is a convenience Transform wrapping Top.PerKey.

Similarly, the Smallest variant also does not have a reverse argument.

beam/sdks/python/apache_beam/transforms/combiners.py

Line 305 in cbe2e0f

def Smallest(pcoll, n, has_defaults=True, key=None):

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

[codecov]

Codecov Report

Merging #27621 (a87de39) into master (cbe2e0f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #27621   +/-   ##
=======================================
  Coverage   71.09%   71.09%           
=======================================
  Files         859      859           
  Lines      104545   104545           
=======================================
+ Hits        74323    74326    +3     
+ Misses      28664    28661    -3     
  Partials     1558     1558           

Flag	Coverage Δ
python	80.28% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sdks/python/apache_beam/transforms/combiners.py	93.43% <100.00%> (ø)

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[github-actions]

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @AnandInguva for label python.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

[AnandInguva]

Do you think it was kept to preserve backward compatibility?

Let's add a note in the changes.md if possible

[Ruwann]

I think it was a mistake when adding the reverse argument, based on the following commit: 866a09d#diff-c939f20e41fc848a26c5c8435fb348f118e81d9ced53dec490c1e5720a1be339L215

I can add a note in changes.md, although I'm not sure where's the best place: in theory, it's a potentially breaking if anyone is calling that function with that argument (even though it doesn't do anything) but that feels a bit heavy for this change. Wdyt?

[github-actions]

Reminder, please take a look at this pr: @AnandInguva

[AnandInguva]

I would leave it like this if it can be a breaking change. We also have a use_fastavro

beam/sdks/python/apache_beam/io/avroio.py

Line 141 in ab93fb1

use_fastavro (bool): This flag is left for API backwards compatibility

which is not used anymore but it was not removed for backwards compatibility. May be add a deprecated warning for 2.50.0 and remove it in 2.51.0?

[github-actions]

Reminder, please take a look at this pr: @AnandInguva

[github-actions]

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @tvalentyn for label python.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

[Ruwann]

Thanks for the feedback, I can update the PR to just add this in the docstring and keep the function signature the same for backwards compatibility.

[tvalentyn]

LGTM