Skip to content

CLN: Made final changes to /pandas/tests/scalar #36388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

Abishek15592
Copy link

No description provided.

@dsaxton
Copy link
Member

dsaxton commented Sep 15, 2020

If I understand correctly the idea of #35925 is to remove commas at the ends of single lines, not trailing commas generally

cc @MarcoGorelli

@dsaxton dsaxton changed the title Made final changes to /pandas/tests/scalar CLN: Made final changes to /pandas/tests/scalar Sep 16, 2020
@dsaxton dsaxton added the Clean label Sep 16, 2020
@ivanovmg
Copy link
Member

If I understand correctly the idea of #35925 is to remove commas at the ends of single lines, not trailing commas generally

cc @MarcoGorelli

Yes, I believe that the magic comma in the end of the multiline statements does not lead to any rearrangement in black. In the single line, however, the situation is different.

https://github.com/psf/black/blob/master/docs/the_black_code_style.md#the-magic-trailing-comma

So, in the case of removing a trailing comma from multiline statements, black would in contrast try to squeeze them into the single line.

@MarcoGorelli
Copy link
Member

Workflow here should be:

  • upgrade black
  • try running black on files
  • for lines that have changed, try removing trailing commas
  • run black again
  • downgrade black to current version
  • run black again to check it doesn't modify anything

@Abishek15592
Copy link
Author

I tried removing the trailing commas present in the test_arithmetic.py file. But running the file throws the below error

No name 'offsets' in module 'pandas._libs.tslibs'

Is this something that I should modify?

@ivanovmg
Copy link
Member

I tried removing the trailing commas present in the test_arithmetic.py file. But running the file throws the below error

No name 'offsets' in module 'pandas._libs.tslibs'

Is this something that I should modify?

Maybe you want to locally re-build C extensions.

python setup.py build_ext --inplace -j 4

https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#creating-a-python-environment-pip

@Abishek15592
Copy link
Author

I tried removing the trailing commas present in the test_arithmetic.py file. But running the file throws the below error
No name 'offsets' in module 'pandas._libs.tslibs'
Is this something that I should modify?

Maybe you want to locally re-build C extensions.

python setup.py build_ext --inplace -j 4

https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#creating-a-python-environment-pip

The error still persists even after performing a local rebuild of C extensions. Also I tried finding out where exactly the linting check had failed in the CI Checks option, but din't find what the linting error was. So could you guide me on that?

@ivanovmg
Copy link
Member

I tried removing the trailing commas present in the test_arithmetic.py file. But running the file throws the below error
No name 'offsets' in module 'pandas._libs.tslibs'
Is this something that I should modify?

Maybe you want to locally re-build C extensions.

python setup.py build_ext --inplace -j 4

https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#creating-a-python-environment-pip

The error still persists even after performing a local rebuild of C extensions. Also I tried finding out where exactly the linting check had failed in the CI Checks option, but din't find what the linting error was. So could you guide me on that?

Apparently, black reformatted files.

black --version
black, version 19.10b0
Checking black formatting
would reformat /home/runner/work/pandas/pandas/pandas/tests/scalar/test_na_scalar.py
would reformat /home/runner/work/pandas/pandas/pandas/tests/scalar/timestamp/test_arithmetic.py
would reformat /home/runner/work/pandas/pandas/pandas/tests/scalar/timestamp/test_constructors.py
Oh no! 💥 💔 💥
3 files would be reformatted, 1130 files would be left unchanged.

Please try to follow the approach by @MarcoGorelli.

@MarcoGorelli
Copy link
Member

But running the file throws the below error

You don't need to run the file, you just need to run black on the file

@simonjayhawkins
Copy link
Member

Thanks @Abishek15592 for the PR. #35925 has been now been closed off by #36493 so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants