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

Black incorrectly formatting code - output has lines with more than 88 characters #1161

Closed
jamespfennell opened this issue Nov 17, 2019 · 9 comments
Labels
F: linetoolong Black makes our lines too long T: style What do we want Blackened code to look like?

Comments

@jamespfennell
Copy link

Describe the bug Black is failing to format a Python snippet to conform with its own rules; in particular, the default 88 character limit is breached.

To Reproduce

  1. Create a file with this Python code:
exclude_because_before = exclude_trips_before is not None and trip_stop_time.get_time().timestamp() <= current_time - float(
    exclude_trips_before
)
  1. Run Black on it with no arguments.
  2. Observe that nothing happens, even though the first line has 125 characters and so should be reformatted.

Expected behavior

The code is reformatted and all lines have 88 or fewer characters.

Environment (please complete the following information):

  • Version: black-19.10b0
  • OS and Python version: Mac OS Mojava 10.14.6 / Python 3.7

Does this bug also happen on master? Yes (checked using website)

@jamespfennell jamespfennell added the T: bug Something isn't working label Nov 17, 2019
@pauloalem
Copy link

Sometime black is unable to reformat the code to conform to the specified line length, take a look
https://github.com/psf/black#line-length
So there's not much you can do in these cases other than tweaking the the code manually to have better output or adding #fmt: off #fmt: on if you do not want black to change it

@jamespfennell
Copy link
Author

Thanks for your response!

I have a new data point though which to me indicates it could be a bug, but maybe I misunderstood Black.

If I manually reformat the above by putting in a single parentheses around the current_time - float(exclude_trips_before) portion, Black then re-formats it to:

exclude_because_before = exclude_trips_before is not None and (
    trip_stop_time.get_time().timestamp()
    <= (current_time - float(exclude_trips_before))
)

which looks Black-like to me. I thought Black was ideally supposed to re-format irrespective of redundant parentheses?

@vemel
Copy link
Contributor

vemel commented Nov 20, 2019

There is a very easy fix, but it changes output dramatically. with fix enabled, we check if first line of right_hand_split fits to line_length as well:

# somewhere in right_hand_split function
        and (
            not can_be_split(head)
            or is_line_short_enough(head, line_length=line_length)
        )

Now, output changes

# old
normal_name = but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying(
    arg1, arg2, arg3
)

this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = [
    1,
    2,
    3,
]

for key in """
    hostname
    port
    username
""".split():
    pass

# new
normal_name = (
    but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying(
         arg1, arg2, arg3
    )
)

this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = [
    1, 2, 3
]

for key in (
    """
    hostname
    port
    username
""".split()
):
    pass

@jaydamask
Copy link

I, too, am seeing lines rendered very long, well beyond PEP8 recommendations.

With line-length = 80 (could be anything reasonable), my PEP8 line renders as

    inline_slope_time_series = impulse_response_builder_tools. \
        compute_noncausal_symmetric_first_difference(
            reference_time_series
    )

However, with black I have

    inline_slope_time_series = impulse_response_builder_tools.compute_noncausal_symmetric_first_difference(
        reference_time_series
    )

Do scroll to the right... it's long. While I'm all for black (I just blackified by codebase), this one is pretty wonky. black's reluctance to use \ chars is fine, but respecting line lengths (-ish) should strongly contribute to the solver's figure-of-merit calculation.

System details are:

  • Python 3.7.5
  • black, version 19.3b0
  • Mac OS 10.15.1 (19B88)

I just confirmed this behavior on your black tip site.

Issues that are related, to greater or lesser degrees, appear to be: #1094, #951, #933, #917, #809, #664 and #510.

@rben01
Copy link

rben01 commented Feb 22, 2021

Not sure if I should make this a separate issue but I'm also seeing this type of issue with dicts whose strings and values are both very long string literals, and "key":"value" is longer than the max line length but "key":\n"value" would not be -- black does not break the line at the colon, even though it should.

@JelleZijlstra JelleZijlstra added F: linetoolong Black makes our lines too long T: style What do we want Blackened code to look like? and removed T: bug Something isn't working labels May 30, 2021
@DANIELF01
Copy link

Not sure if I should make this a separate issue but I'm also seeing this type of issue with dicts whose strings and values are both very long string literals, and "key":"value" is longer than the max line length but "key":\n"value" would not be -- black does not break the line at the colon, even though it should.

I am seeing this too. Black formatted output below:

def run_postgres_job(postgres: Postgres, neo4j: Neo4j):
    tmp_folder = "/var/tmp/amundsen/table_metadata"
    node_files_folder = f"{tmp_folder}/nodes/"
    relationship_files_folder = f"{tmp_folder}/relationships/"

    job_config = ConfigFactory.from_dict(
        {
            f"extractor.postgres_metadata.{PostgresMetadataExtractor.USE_CATALOG_AS_CLUSTER_NAME}": False,
            f"extractor.postgres_metadata.{PostgresMetadataExtractor.CLUSTER_KEY}": Postgres.host.split(
                "."
            )[
                0
            ],
            f"extractor.postgres_metadata.extractor.sqlalchemy.{SQLAlchemyExtractor.CONN_STRING}": postgres.connection_string,
            f"loader.filesystem_csv_neo4j.{FsNeo4jCSVLoader.NODE_DIR_PATH}": node_files_folder,
            f"loader.filesystem_csv_neo4j.{FsNeo4jCSVLoader.RELATION_DIR_PATH}": relationship_files_folder,
            f"loader.filesystem_csv_neo4j.{FsNeo4jCSVLoader.SHOULD_DELETE_CREATED_DIR}": True,
            f"publisher.neo4j.{neo4j_csv_publisher.NODE_FILES_DIR}": node_files_folder,
            f"publisher.neo4j.{neo4j_csv_publisher.RELATION_FILES_DIR}": relationship_files_folder,
            f"publisher.neo4j.{neo4j_csv_publisher.NEO4J_END_POINT_KEY}": neo4j.endpoint,
            f"publisher.neo4j.{neo4j_csv_publisher.NEO4J_USER}": neo4j.username,
            f"publisher.neo4j.{neo4j_csv_publisher.NEO4J_PASSWORD}": neo4j.password,
            f"publisher.neo4j.{neo4j_csv_publisher.JOB_PUBLISH_TAG}": "unique_tag",  # should use unique tag here like {ds}
        }
    )
    job = DefaultJob(
        conf=job_config,
        task=DefaultTask(
            extractor=PostgresMetadataExtractor(), loader=FsNeo4jCSVLoader()
        ),
        publisher=Neo4jCsvPublisher(),
    )
    return job

@JelleZijlstra
Copy link
Collaborator

@DANIELF01 that looks different from the original report. Please check if any of the issues in F: linetoolong Black makes our lines too long match your report and if not, open a new issue.

@DANIELF01
Copy link

Will do.

@JelleZijlstra
Copy link
Collaborator

The original example is now wrapped correctly:

% black -c '''exclude_because_before = exclude_trips_before is not None and trip_stop_time.get_time().timestamp() <= current_time - float(
    exclude_trips_before
)
'''
exclude_because_before = (
    exclude_trips_before is not None
    and trip_stop_time.get_time().timestamp()
    <= current_time - float(exclude_trips_before)
)

Some other messages in this thread apply to other issues in F: linetoolong Black makes our lines too long

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: linetoolong Black makes our lines too long T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

7 participants