Skip to content

black_formatting setting is ignored #1122

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

Merged
merged 2 commits into from
Sep 29, 2022

Conversation

jmoralez
Copy link
Contributor

The black_formatting option is now calling str2bool from fastcore so it's value when retrieved by cfg.get('black_formatting') is either 0 or 1, so the code path that does the black formatting is currently inaccessible. This changes the comparison to check against 1.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@seeM
Copy link
Contributor

seeM commented Sep 29, 2022

Thanks for the contribution! 😄 This can be simplified a little since 1 is "truthy" and 0 is "falsy". We also don't need to provide the default here anymore, they're now defined in _apply_defaults here. So you could do:

if (not cfg.black_formatting and not force) or cell.cell_type != 'code': return

@seeM seeM added the bug Something isn't working label Sep 29, 2022
@seeM seeM changed the title compare black_formatting value from config against the value 1 black_formatting setting is ignored Sep 29, 2022
@jmoralez
Copy link
Contributor Author

Thanks for the suggestion! Applied in 19a95dd

@seeM seeM merged commit eac9c9d into AnswerDotAI:master Sep 29, 2022
@jmoralez jmoralez deleted the black-formatting-bool branch September 29, 2022 03:04
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

Successfully merging this pull request may close these issues.

2 participants