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

Formatting of if condition can be improved. #649

Closed
sainadh-d opened this issue Dec 24, 2018 · 6 comments
Closed

Formatting of if condition can be improved. #649

sainadh-d opened this issue Dec 24, 2018 · 6 comments
Labels
F: parentheses Too many parentheses, not enough parentheses, and so on. R: duplicate This issue or pull request already exists T: enhancement New feature or request

Comments

@sainadh-d
Copy link

sainadh-d commented Dec 24, 2018

Operating system: Mac OS X Mojave
Python version: Python 3.6.7
Black version: 18.9b0
Does also happen on master: Yes

I have a piece of code which looks like

class MyClass:
    def my_func(self, check=False):
        flag = False
        try:
            configuration = self.get_config_from_db()
            if check:
                if 'expected_flavour' in configuration.get('flavours', []) and app_settings[0].upper() in configuration.get('settings', []):
                    flag = True
        except:
            pass
        return flag

Upon running black on this

black -S -l 120 temp.py

I get

class MyClass:
    def my_func(self, check=False):
        flag = False
        try:
            configuration = self.get_config_from_db()
            if check:
                if 'expected_flavour' in configuration.get('flavours', []) and app_settings[
                    0
                ].upper() in configuration.get('settings', []):
                    flag = True
        except:
            pass
        return flag

Formatting of the if condition above is not readable.

if 'expected_flavour' in configuration.get('flavours', []) and app_settings[
      0
].upper() in configuration.get('settings', []):

IMO the following would be more readable, which is done when there are 3 or more checks in if

class MyClass:
    def my_func(self, check=False):
        flag = False
        try:
            configuration = self.get_config_from_db()
            if check:
                if (
                    'expected_flavour' in configuration.get('flavours', [])
                    and app_settings[0].upper() in configuration.get('settings', [])
                ):
                    flag = True
        except:
            pass
        return flag
@sainadh-goibibo
Copy link

@JelleZijlstra @ambv Pls check

@JelleZijlstra
Copy link
Collaborator

I agree that the formatting with index expressions on their own line isn't great and that your proposed formatting looks nicer in this case. I haven't looked at how realistic it is to implement in general though, and maybe this change would lead to worse formatting in some other case.

@zsol zsol added T: enhancement New feature or request F: parentheses Too many parentheses, not enough parentheses, and so on. labels Feb 14, 2019
@ambv
Copy link
Collaborator

ambv commented Mar 4, 2020

We should at least try to make Black not choose to split on indexes if optional parentheses can yield nicer formattings.

@MikeFalowski
Copy link

Still happen in version 19. It would be really nice improvement.

@miriaford
Copy link

Could any contributor please point us to how we can modify this behavior ourselves? I'm willing to fork Black and hack the source code.

@felix-hilden
Copy link
Collaborator

I believe this is essentially the same request as the newer #2156, which has some more discussion (though it was initially rejected) and associated PRs. I'll close this issue in favor of it!

@felix-hilden felix-hilden added the R: duplicate This issue or pull request already exists label Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: parentheses Too many parentheses, not enough parentheses, and so on. R: duplicate This issue or pull request already exists T: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants