HelpersTask397_Replace_data_with_df_in_hdataframe

[aangelo9]

Related to issue #397

This PR renames all usages of data → df in helpers/hdataframe.py where the variable represents a Pandas DataFrame, to improve clarity.

• Updated function arguments and internal references
• Verified all logic remains the same, only refactoring

[aangelo9]

Run_linter and run_fast_tests failed.

For Run_linter:

Error states:

1. Unpacking a string is disallowed

helpers/hdataframe.py:111: error: Unpacking a string is disallowed [misc] [mypy]

In hdataframe.py, Refers to line:

for comparison_method, val in tuple_:

2. Git Glient Not Clean

AssertionError: The Git client is not clean: helpers/hdataframe.py

Solution:

For error 1, we could:

Strengthen the type check:

if not isinstance(tuple_[0], tuple):
    hdbg.dassert_isinstance(tuple_, tuple)
    tuple_ = (tuple_,)

or

if isinstance(tuple_, tuple) and all(isinstance(x, str) for x in tuple_):
    tuple_ = (tuple_,)

For error 2: Likely will be fixed when error 1 is fixed.

For run_fast_tests

Under TestFilterDataByMethod, the test is calling:

act = hdatafr.filter_data_by_method(
            data=data, filters=filters, mode=mode, info=info
        )

But since the function is refactored, it should be:

act = hdatafr.filter_data_by_method(
            df=df, filters=filters, mode=mode, info=info
        )

[aangelo9]

Should I proceed with the test fixes?

[sonniki]

Should I proceed with the test fixes?

Of course. In the future, go ahead and fix the failing checks before requesting review. Also, you don't have to provide a full report of the failing tests, etc -- it's all part of debugging that you are expected to carry out before you present the "finished product" to the reviewers.

BTW the Linter check more likely fails because when you run i lint on the file, it modifies it (fixes formatting, etc), and these modifications are not checked in here. I.e. try to run i lint on the file again, it will automatically stage the file for commit, and then you commit and push it. Other Linter complaints, like the mypy one, are nice to fix but not absolutely required in this case.

[samarth9008]

Of course. In the future, go ahead and fix the failing checks before requesting review. Also, you don't have to provide a full report of the failing tests, etc -- it's all part of debugging that you are expected to carry out before you present the "finished product" to the reviewers.

@sonniki I have been seeing this behavior from last year. People focus more on writing the code and requesting for review rather than checking if their work is correct. We should update the doc and add something like "make sure tests are green before requesting for a review".

[aangelo9]

All tests are green. Awaiting review.

[sonniki]

@aangelo9 pls also make sure to resolve conversations after you've addressed them and keep the branch up to date with master (you can use "Update branch" button on the PR page).