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

ENH: slightly prefer split before named assigns #692

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

apbard
Copy link
Contributor

@apbard apbard commented Mar 19, 2019

Following up #691, this is an attempt to fix it by slightly favour split before named assign.
In the current state it only mitigates the effect in some cases:

# now ok
def fooooooooooooo(aka=['aaaaaaaa', 'aaaaaaa', 'aaaa', 'aa', 'a'],
                   types=['double', 'bool'],
                   validator=some_looooooooooong_name):
    pass

# still bad
def fooooooooooooo(type01=['double'], type02=['double'], type03=['double'],
                   aka=['aaaaaaaa', 'aaaaaaa', 'aaaa', 'aa',
                        'a'], AAAAA=['double', 'bool'],
                   validator=some_looooooooooong_name):
    pass

Also it breaks a couple of tests that are right the way they are now. Example:

  def f():
    if True:
      pytree_utils.InsertNodesBefore(
          _CreateCommentsFromPrefix(
              comment_prefix, comment_lineno, comment_column,
              standalone=True), ancestor_at_indent)  

@gwelymernans Any idea where/how to fix this?

@bwendling
Copy link
Member

Look around format_decision_state.py:341. That bit of code is trying to split before all named assigns. That bit of code probably isn't triggered for the cases above for some reason (could just be an option isn't set). I think you'll find that either you can modify that bit of code or gain some insight from it on how to address this.

@apbard
Copy link
Contributor Author

apbard commented Mar 22, 2019

I am not sure we can do that there. Because ideally what we would like to achieve is something like:
"if we have to split somewhere, it is better to split before an argument than in the middle of a list"
In the function _MustSplit we can just force the splitting, right?

Base automatically changed from master to main January 23, 2021 06:09
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.

3 participants