modin icon indicating copy to clipboard operation
modin copied to clipboard

FIX-#4099: Convert columns to list of str

Open AndreyPavlenko opened this issue 3 years ago • 3 comments

What do these changes do?

The method Table.rename_columns() accepts a list of strings, but not numbers, thus, the columns must be converted to strings.

  • [x] commit message follows format outlined here
  • [x] passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
  • [x] passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
  • [x] signed commit with git commit -s
  • [x] Resolves #4099
  • [ ] tests added and passing
  • [ ] module layout described at docs/development/architecture.rst is up-to-date
  • [ ] added (Issue Number: PR title (PR Number)) and github username to release notes for next major release

AndreyPavlenko avatar Aug 03 '22 23:08 AndreyPavlenko

Codecov Report

Merging #4767 (3e45f2d) into master (f7fd559) will decrease coverage by 0.53%. The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #4767      +/-   ##
==========================================
- Coverage   85.37%   84.84%   -0.54%     
==========================================
  Files         265      266       +1     
  Lines       19622    19908     +286     
==========================================
+ Hits        16752    16890     +138     
- Misses       2870     3018     +148     
Impacted Files Coverage Δ
...entations/omnisci_on_native/dataframe/dataframe.py 91.81% <88.88%> (-0.34%) :arrow_down:
modin/experimental/sklearn/__init__.py 0.00% <0.00%> (-100.00%) :arrow_down:
modin/experimental/xgboost/test/test_dmatrix.py 0.00% <0.00%> (-100.00%) :arrow_down:
modin/experimental/xgboost/test/test_xgboost.py 0.00% <0.00%> (-100.00%) :arrow_down:
modin/experimental/core/execution/ray/__init__.py 0.00% <0.00%> (-100.00%) :arrow_down:
...n/experimental/sklearn/model_selection/__init__.py 0.00% <0.00%> (-100.00%) :arrow_down:
...mental/sklearn/model_selection/train_test_split.py 0.00% <0.00%> (-100.00%) :arrow_down:
...tal/core/execution/ray/implementations/__init__.py 0.00% <0.00%> (-100.00%) :arrow_down:
...ecution/ray/implementations/pandas_on_ray/io/io.py 0.00% <0.00%> (-100.00%) :arrow_down:
...tion/ray/implementations/pandas_on_ray/__init__.py 0.00% <0.00%> (-100.00%) :arrow_down:
... and 61 more

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

codecov[bot] avatar Aug 04 '22 14:08 codecov[bot]

@AndreyPavlenko thanks for contribution! Please also add a test and a release note into docs\release_notes\release_notes-0.16.0.rst.

anmyachev avatar Aug 05 '22 10:08 anmyachev

The fix looks general and may even be fixing the more general related issue (#3370), so I would suggest trying to add general testing for integer column names to see if we can close the #3370 with the fix, what do you think?

Sorry, but looking deeper, now I'm not sure that the fix is quite correct, because it changes the type of the column names.

AndreyPavlenko avatar Aug 05 '22 14:08 AndreyPavlenko

I think, the issue #3370 is also resolved by this fix. The test test_set_index() covers this case.

AndreyPavlenko avatar Aug 17 '22 09:08 AndreyPavlenko

@AndreyPavlenko, cool, closing that issue as well.

YarShev avatar Aug 17 '22 09:08 YarShev