Skip to content
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

Inconsistent output for code quality checks #39

Closed
stephenbach opened this issue May 31, 2021 · 7 comments
Closed

Inconsistent output for code quality checks #39

stephenbach opened this issue May 31, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@stephenbach
Copy link
Member

See this for an example: bc0a8f1

make style says that it is reformatting promptsource/promptsource.py but there is no diff afterwards. Rerunning make style produces the same output and make quality fails, saying that it would be reformatted.

@tianjianjiang
Copy link
Contributor

I will investigate it.
(IIRC I encountered a similar situation, but since the make style/quality commands are copied from HuggingFace so I figure that would be a low priority issue.)

@stephenbach
Copy link
Member Author

Thanks! Some more info:

This is the diff it wants

black --diff --line-length 119 --target-version py38 promptsource
--- promptsource/promptsource.py	2021-05-31 15:07:06.548574 +0000
+++ promptsource/promptsource.py	2021-05-31 15:08:40.062640 +0000
@@ -1,11 +1,10 @@
 import datasets
 import requests
 import streamlit as st
 from session import _get_state
-from utils import (_ADDITIONAL_ENGLISH_DATSETS, removeHyphen,
-                   renameDatasetColumn)
+from utils import _ADDITIONAL_ENGLISH_DATSETS, removeHyphen, renameDatasetColumn
 
 from templates import Template, TemplateCollection

When I make that change manually, and then run make style it says 5 files left unchanged BUT it actually changes promptsource.py back to the 2-line import and we're back in the same state.

The only change I've made to your initial config @tianjianjiang is that I added --max-line-length 119 to flake8 because it seemed inconsistent with the black config.

@tianjianjiang
Copy link
Contributor

Thanks! Some more info:

When I make that change manually, and then run make style it says 5 files left unchanged BUT it actually changes promptsource.py back to the 2-line import and we're back in the same state.

I suspect that's an unwelcome side effect from isort, I will look into isort's options.

The only change I've made to your initial config @tianjianjiang is that I added --max-line-length 119 to flake8 because it seemed inconsistent with the black config.

That is simplified from datasets and transformers but admittedly the latter has a set of more sophisticated formatting routines that could be more precise and consistent. I will also look into it.

@VictorSanh
Copy link
Member

Yes, if i remember correctly, there is some inconsistency between black and isort import conventions -> PyCQA/isort#1518
Don't have a solution on top of my head, until we find it, let's ignore tests and merge @stephenbach

@arnaudstiegler
Copy link
Contributor

If the problem is from a black/isort compatibility, I believe we can fix that using this:
https://pycqa.github.io/isort/docs/configuration/black_compatibility/

You define a config file for isort and have "black" for the profile

@VictorSanh
Copy link
Member

@tianjianjiang after reading @arnaudstiegler's comment, you also need to import the setup.cfg file!
I fixed it in commit c1ec687 and the tests seem to be back to normal

@tianjianjiang
Copy link
Contributor

@VictorSanh @arnaudstiegler Thank you guys and sorry for the late reaction.
BTW, I was thinking at least we can move isort to the top and introduce autoflake for following flake8 recommendations, but just something less urgent I suppose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants