[Bug] fix fillna function on a single column fail

[DKER2]

fixes #31855
In fillna to expression fuction in here, it add ConstantExpression as args to fillna() if value is constant. This would cause problem in expression to compute stage function as in this line all args will be added as inputs to compute stage. This constant expression will latter be computed by this function, but as it is a constant expression, it do not have any input that will make the program fail. We only should add ComputedExpression and PlaceHolderExpression

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

[github-actions]

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

R: @shunping 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).

[shunping]

Thanks for fixing this bug, @DKER2 . Could you also add a unit test (something the following) in to your PR?

import tempfile

import apache_beam as beam
from apache_beam.dataframe import convert
from apache_beam.testing.util import assert_that
from apache_beam.testing.util import equal_to

with tempfile.NamedTemporaryFile(mode="w") as fp:
  fp.writelines(("col1,col2\n", "1,1\n", "NaN,1\n", "-1,1\n"))
  fp.flush()

  pipeline = beam.Pipeline(None)
  beam_df = pipeline | 'Read CSV' >> beam.dataframe.io.read_csv(fp.name)
  beam_df['col1'] = beam_df['col1'].fillna(0)
  assert_that(
    convert.to_pcollection(beam_df),
    equal_to([(1,1), (0, 1),(-1, 1)]))

[shunping]

@tvalentyn

[DKER2]

Could you please check the name of the test and confirm if the file where I've placed it is correct?

[tvalentyn]

I would place it together with other fillna tests:

beam/sdks/python/apache_beam/dataframe/frames_test.py

Line 987 in deb4b4a

def test_fillna_columns(self):

[DKER2]

I have tried to put in that file like this commit , but seem that this function

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

Line 362 in deb4b4a

def expr_to_stages(expr):

is not triggered, the test triggered different flow. And the test pass even with the old version of code.

[DKER2]

Please help to advise

[shunping]

Sorry for the late reply.

The bug is only triggered in the transform of DeferredDataFrame, so you have to put that in a pipeline so that when the pipeline runs the corresponding transform code path will run.

Could you put a test like the following into frames_test.py?

import numpy as np

import apache_beam as beam
from apache_beam.dataframe import convert
from apache_beam.testing.util import assert_that
from apache_beam.testing.util import equal_to

with beam.Pipeline(None) as p:
  pcoll = (p | beam.Create([1.0, np.nan, -1.0]) | beam.Select(x=lambda x: x))
  df = convert.to_dataframe(pcoll)
  df_new = df['x'].fillna(0)
  assert_that(
    convert.to_pcollection(df_new),
    equal_to([1.0, 0.0, -1.0]))

[DKER2]

Thank you @shunping for your suggestion. That works as well

[github-actions]

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

[shunping]

Could you please add the test? Thanks!

[DKER2]

Added Tests

[shunping]

LGTM

[github-actions]

R: @damccorm for final approval

[damccorm]

Thanks!