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

Formatter instability: lines after comment at module level #7604

Closed
charliermarsh opened this issue Sep 22, 2023 · 3 comments · Fixed by #7607
Closed

Formatter instability: lines after comment at module level #7604

charliermarsh opened this issue Sep 22, 2023 · 3 comments · Fixed by #7607
Assignees
Labels
bug Something isn't working formatter Related to the formatter

Comments

@charliermarsh
Copy link
Member

charliermarsh commented Sep 22, 2023

Given:

import random;

# Defaults for arguments are defined here
# args.threshold = None;


logger = logging.getLogger("FastProject");

We're reformatting as:

# -*- coding: utf-8 -*-
import random

# Defaults for arguments are defined here
# args.threshold = None;


logger = logging.getLogger("FastProject")

Formatted twice:

import random

# Defaults for arguments are defined here
# args.threshold = None;

logger = logging.getLogger("FastProject")
--- Formatted once
+++ Formatted twice
@@ -3,5 +3,4 @@
 # Defaults for arguments are defined here
 # args.threshold = None;
 
-
 logger = logging.getLogger("FastProject")

(That is: removing a newline before logger =.)

This leads to an instability.

Sourced from #7590 (A_3703355012074323529.py).

@charliermarsh charliermarsh added bug Something isn't working formatter Related to the formatter labels Sep 22, 2023
@charliermarsh charliermarsh added this to the Formatter: Beta milestone Sep 22, 2023
@charliermarsh
Copy link
Member Author

For posterity, the original source code here (which doesn't strip the newline) is:

# -*- coding: utf-8 -*-
"""Contains information that is useful to share
across all modules.

Right now, mainly command-line arguments, logging
and resource locations.
"""
from __future__ import absolute_import, print_function, division;
import logging;
import sys
import os
import random;

# Defaults for arguments are defined here
# These are all overwritten if called from the command line
# However, by putting defaults here, it makes it easier to
#    call FastProject from within other scripts
# These should be kept in sync with arguments in __main__.py
# args = namedtuple("defalt_args",  ["data_file",  "housekeeping",
#                                    "signatures",  "precomputed",  "output",
#                                    "nofilter",  "nomodel",  "pca_filter",
#                                    "qc",  "subsample_size",
#                                    "min_signature_genes",  "projections",
#                                    "weights",  "threshold"]);
# 
# args.data_file = "";
# args.housekeeping = "";
# args.signatures = [];
# args.precomputed = [];
# args.output = "";
# args.nofilter = False;
# args.nomodel = False;
# args.pca_filter = False;
# args.qc = False;
# args.subsample_size = None;
# args.min_signature_genes = 5;
# args.projections = [];
# args.weights = "";
# args.threshold = None;


logger = logging.getLogger("FastProject");

So everything is suffixed with a semicolon.

@MichaReiser
Copy link
Member

This seems to be fixed on main (creating a snapshot test and running our test suite passes without an instability error)

@MichaReiser
Copy link
Member

Okay, the case with semicolons isn't fixed ;)

@charliermarsh charliermarsh self-assigned this Sep 22, 2023
charliermarsh added a commit that referenced this issue Sep 22, 2023
…rts (#7607)

## Summary

Given:

```python
# -*- coding: utf-8 -*-
import random

# Defaults for arguments are defined here
# args.threshold = None;


logger = logging.getLogger("FastProject")
```

We want to count the number of newlines after `import random`, to ensure
that there's _at least one_, but up to two.

Previously, we used the end range of the statement (then skipped
trivia); instead, we need to use the end of the _last comment_. This is
similar to #7556.

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

Successfully merging a pull request may close this issue.

2 participants