Closed parthosa closed 2 months ago
I noticed a couple of weird things that will be nice to be looked at just in case they don’t turn to be real bugs:
A statement that has no effect here (Line 824): https://github.com/NVIDIA/spark-rapids-tools/blob/35edbb5a348ad5e22cd2273cfce51484[…]5c3/user_tools/src/spark_rapids_tools/tools/qualx/preprocess.py
Local variable ‘stages_supp’ might be referenced before assignment (Line 992): https://github.com/NVIDIA/spark-rapids-tools/blob/35edbb5a348ad5e22cd2273cfce51484[…]5c3/user_tools/src/spark_rapids_tools/tools/qualx/preprocess.py
scan_tbl defined to return dataFrame but the variable out returned at the end is actually a dict . Expected type ‘List[Mapping[str, DataFrame]]’, got ‘Dict[str, Union[DataFrame, Any]]’ instead (Line 1084): https://github.com/NVIDIA/spark-rapids-tools/blob/35edbb5a348ad5e22cd2273cfce51484[…]5c3/user_tools/src/spark_rapids_tools/tools/qualx/preprocess.py
I noticed a couple of weird things that will be nice to be looked at just in case they don’t turn to be real bugs:
- A statement that has no effect here (Line 824): https://github.com/NVIDIA/spark-rapids-tools/blob/35edbb5a348ad5e22cd2273cfce51484[…]5c3/user_tools/src/spark_rapids_tools/tools/qualx/preprocess.py
This statement seems to be removed in the latest version
- Local variable ‘stages_supp’ might be referenced before assignment (Line 992): https://github.com/NVIDIA/spark-rapids-tools/blob/35edbb5a348ad5e22cd2273cfce51484[…]5c3/user_tools/src/spark_rapids_tools/tools/qualx/preprocess.py
Will add an empty df with required columns before the if
branching
- scan_tbl defined to return dataFrame but the variable out returned at the end is actually a dict . Expected type ‘List[Mapping[str, DataFrame]]’, got ‘Dict[str, Union[DataFrame, Any]]’ instead (Line 1084): https://github.com/NVIDIA/spark-rapids-tools/blob/35edbb5a348ad5e22cd2273cfce51484[…]5c3/user_tools/src/spark_rapids_tools/tools/qualx/preprocess.py
Will fix the return type
Post QualX migration, the
qualx
source code is excluded from linters and tests (tox, pytest, flake8). We need to refactor QualX to comply with these and add unit tests if required.cc: @leewyang