Allow newlines after function headers without docstrings

[ntBre]

Summary

This is a first step toward fixing #9745. After reviewing our open issues and several Black issues and PRs, I personally found the function case the most compelling, especially with very long argument lists:

def func(
	self,
	arg1: int,
	arg2: bool,
	arg3: bool,
	arg4: float,
	arg5: bool,
) -> tuple[...]:
	if arg2 and arg3:
		raise ValueError

or many annotations:

def function(
    self, data: torch.Tensor | tuple[torch.Tensor, ...], other_argument: int
) -> torch.Tensor | tuple[torch.Tensor, ...]:
    do_something(data)
    return something

I think docstrings help the situation substantially both because syntax highlighting will usually give a very clear separation between the annotations and the docstring and because we already allow a blank line after the docstring:

def function(
    self, data: torch.Tensor | tuple[torch.Tensor, ...], other_argument: int
) -> torch.Tensor | tuple[torch.Tensor, ...]:
    """
	A function doing something.

	And a longer description of the things it does.
	"""

    do_something(data)
    return something

There are still other comments on #9745, such as this one with 9 upvotes, where users specifically request blank lines in all block types, or at least including conditionals and loops. I'm sympathetic to that case as well, even if personally I don't find an example like this:

if blah:

    # Do some stuff that is logically related
    data = get_data()

    # Do some different stuff that is logically related
    results = calculate_results()

    return results

to be much more readable than:

if blah:
    # Do some stuff that is logically related
    data = get_data()

    # Do some different stuff that is logically related
    results = calculate_results()

    return results

I'm probably just used to the latter from the formatters I've used, but I do prefer it. I also think that functions are the least susceptible to the accidental introduction of a newline after refactoring described in Micha's comment on #8893.

I actually considered further restricting this change to functions with multiline headers. I don't think very short functions like:

def foo():

    return 1

benefit nearly as much from the allowed newline, but I just went with any function without a docstring for now. I guess a marginal case like:

def foo(a_long_parameter: ALongType, b_long_parameter: BLongType) -> CLongType:

    return 1

might be a good argument for not restricting it.

I caused a couple of syntax errors before adding special handling for the ellipsis-only case, so I suspect that there are some other interesting edge cases that may need to be handled better.

Test Plan

Existing tests, plus a few simple new ones. As noted above, I suspect that we may need a few more for edge cases I haven't considered.

[github-actions]

ruff-ecosystem results

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

ℹ️ ecosystem check detected format changes. (+419 -0 lines in 282 files in 18 projects; 37 projects unchanged)

PostHog/HouseWatch (+2 -0 lines across 2 files)

ruff format --preview

housewatch/api/async_migration.py~L52

 
     @action(methods=["POST"], detail=True)
     def trigger(self, request, **kwargs):
+
         migration = self.get_object()
 
         migration.status = MigrationStatus.Starting

housewatch/async_migrations/runner.py~L27

 def start_async_migration(
     migration: AsyncMigration, ignore_posthog_version=False
 ) -> bool:
+
     if migration.status not in [MigrationStatus.Starting, MigrationStatus.NotStarted]:
         logger.error(f"Initial check failed for async migration {migration.name}")
         return False

RasaHQ/rasa (+152 -0 lines across 87 files)

ruff format --preview

data/test_classes/custom_slots.py~L28

         value_reset_delay: Optional[int] = None,
         influence_conversation: bool = True,
     ) -> None:
+
         super().__init__(
             name=name,
             initial_value=initial_value,

examples/reminderbot/actions/actions.py~L28

         tracker: Tracker,
         domain: Dict[Text, Any],
     ) -> List[Dict[Text, Any]]:
+
         dispatcher.utter_message("I will remind you in 5 seconds.")
 
         date = datetime.datetime.now() + datetime.timedelta(seconds=5)

examples/reminderbot/actions/actions.py~L56

         tracker: Tracker,
         domain: Dict[Text, Any],
     ) -> List[Dict[Text, Any]]:
+
         name = next(tracker.get_slot("PERSON"), "someone")
         dispatcher.utter_message(f"Remember to call {name}!")
 

examples/reminderbot/actions/actions.py~L71

     async def run(
         self, dispatcher, tracker: Tracker, domain: Dict[Text, Any]
     ) -> List[Dict[Text, Any]]:
+
         conversation_id = tracker.sender_id
 
         dispatcher.utter_message(f"The ID of this conversation is '{conversation_id}'.")

examples/reminderbot/actions/actions.py~L98

         tracker: Tracker,
         domain: Dict[Text, Any],
     ) -> List[Dict[Text, Any]]:
+
         plant = next(tracker.get_latest_entity_values("plant"), "someone")
         dispatcher.utter_message(f"Your {plant} needs some water!")
 

examples/reminderbot/actions/actions.py~L113

     async def run(
         self, dispatcher, tracker: Tracker, domain: Dict[Text, Any]
     ) -> List[Dict[Text, Any]]:
+
         dispatcher.utter_message("Okay, I'll cancel all your reminders.")
 
         # Cancel all reminders

examples/reminderbot/callback_server.py~L4

 
 
 def create_app() -> Sanic:
+
     bot_app = Sanic("callback_server", configure_logging=False)
 
     @bot_app.post("/bot")

rasa/cli/export.py~L177

 
 
 async def _export_trackers(args: argparse.Namespace) -> None:
+
     _assert_max_timestamp_is_greater_than_min_timestamp(args)
 
     endpoints = rasa.core.utils.read_endpoints_from_path(args.endpoints)

rasa/cli/run.py~L58

 
 
 def _validate_model_path(model_path: Text, parameter: Text, default: Text) -> Text:
+
     if model_path is not None and not os.path.exists(model_path):
         reason_str = f"'{model_path}' not found."
         if model_path is None:

rasa/core/actions/action.py~L658

 
 class RemoteAction(Action):
     def __init__(self, name: Text, action_endpoint: Optional[EndpointConfig]) -> None:
+
         self._name = name
         self.action_endpoint = action_endpoint
 

rasa/core/agent.py~L503

         return await self.handle_message(msg)
 
     def _set_fingerprint(self, fingerprint: Optional[Text] = None) -> None:
+
         if fingerprint:
             self.fingerprint = fingerprint
         else:

rasa/core/channels/botframework.py~L48

         bot: Text,
         service_url: Text,
     ) -> None:
+
         service_url = (
             f"{service_url}/" if not service_url.endswith("/") else service_url
         )

rasa/core/channels/callback.py~L22

         return "callback"
 
     def __init__(self, endpoint: EndpointConfig) -> None:
+
         self.callback_endpoint = endpoint
         super().__init__()
 

rasa/core/channels/facebook.py~L33

         page_access_token: Text,
         on_new_message: Callable[[UserMessage], Awaitable[Any]],
     ) -> None:
+
         self.on_new_message = on_new_message
         self.client = MessengerClient(page_access_token)
         self.last_message: Dict[Text, Any] = {}

rasa/core/channels/facebook.py~L171

         return "facebook"
 
     def __init__(self, messenger_client: MessengerClient) -> None:
+
         self.messenger_client = messenger_client
         super().__init__()
 

rasa/core/channels/facebook.py~L349

     def blueprint(
         self, on_new_message: Callable[[UserMessage], Awaitable[Any]]
     ) -> Blueprint:
+
         fb_webhook = Blueprint("fb_webhook", __name__)
 
         # noinspection PyUnusedLocal

rasa/core/channels/hangouts.py~L40

 
     @staticmethod
     def _text_card(message: Dict[Text, Any]) -> Dict:
+
         card = {
             "cards": [
                 {

rasa/core/channels/hangouts.py~L190

 
     @classmethod
     def from_credentials(cls, credentials: Optional[Dict[Text, Any]]) -> InputChannel:
+
         if credentials:
             return cls(credentials.get("project_id"))
 

rasa/core/channels/hangouts.py~L202

         hangouts_room_added_intent_name: Optional[Text] = "/room_added",
         hangouts_removed_intent_name: Optional[Text] = "/bot_removed",
     ) -> None:
+
         self.project_id = project_id
         self.hangouts_user_added_intent_name = hangouts_user_added_intent_name
         self.hangouts_room_added_intent_name = hangouts_room_added_intent_name

rasa/core/channels/hangouts.py~L224

 
     @staticmethod
     def _extract_sender(req: Request) -> Text:
+
         if req.json["type"] == "MESSAGE":
             return req.json["message"]["sender"]["displayName"]
 

rasa/core/channels/hangouts.py~L231

 
     # noinspection PyMethodMayBeStatic
     def _extract_message(self, req: Request) -> Text:
+
         if req.json["type"] == "MESSAGE":
             message = req.json["message"]["text"]
 

rasa/core/channels/hangouts.py~L290

 
         @custom_webhook.route("/webhook", methods=["POST"])
         async def receive(request: Request) -> HTTPResponse:
+
             if self.project_id:
                 token = request.headers.get("Authorization", "").replace("Bearer ", "")
                 self._check_token(token)

rasa/core/channels/rocketchat.py~L114

         )
 
     def __init__(self, user: Text, password: Text, server_url: Text) -> None:
+
         self.user = user
         self.password = password
         self.server_url = server_url

rasa/core/channels/webexteams.py~L78

         sender_id: Optional[Text],
         metadata: Optional[Dict],
     ) -> Any:
+
         try:
             out_channel = self.get_output_channel()
             user_msg = UserMessage(

rasa/core/featurizers/single_state_featurizer.py~L186

         precomputations: Optional[MessageContainerForCoreFeaturization],
         sparse: bool = False,
     ) -> Dict[Text, List[Features]]:
+
         # Remove entities from possible attributes
         attributes = set(
             attribute for attribute in sub_state.keys() if attribute != ENTITIES

rasa/core/nlg/callback.py~L62

     """
 
     def __init__(self, endpoint_config: EndpointConfig) -> None:
+
         self.nlg_endpoint = endpoint_config
 
     async def generate(

rasa/core/policies/rule_policy.py~L951

     def _find_action_from_loop_happy_path(
         tracker: DialogueStateTracker,
     ) -> Tuple[Optional[Text], Optional[Text]]:
+
         active_loop_name = tracker.active_loop_name
         if active_loop_name is None:
             return None, None

rasa/core/policies/ted_policy.py~L1925

         text_output: tf.Tensor,
         text_sequence_lengths: tf.Tensor,
     ) -> tf.Tensor:
+
         text_transformed, text_mask, text_sequence_lengths = self._reshape_for_entities(
             tf_batch_data,
             dialogue_transformer_output,

rasa/core/policies/ted_policy.py~L2127

         text_output: tf.Tensor,
         text_sequence_lengths: tf.Tensor,
     ) -> Tuple[tf.Tensor, tf.Tensor]:
+
         text_transformed, _, text_sequence_lengths = self._reshape_for_entities(
             tf_batch_data,
             dialogue_transformer_output,

rasa/core/processor.py~L785

     async def _handle_message_with_tracker(
         self, message: UserMessage, tracker: DialogueStateTracker
     ) -> None:
+
         if message.parse_data:
             parse_data = message.parse_data
         else:

rasa/core/test.py~L702

     event: ActionExecuted,
     fail_on_prediction_errors: bool,
 ) -> Tuple[EvaluationStore, PolicyPrediction, Optional[EntityEvaluationResult]]:
+
     action_executed_eval_store = EvaluationStore()
 
     expected_action_name = event.action_name

rasa/core/test.py~L820

     List[Dict[Text, Any]],
     List[EntityEvaluationResult],
 ]:
+
     processor = agent.processor
     if agent.processor is not None:
         processor = agent.processor

rasa/engine/recipes/default_recipe.py~L666

         preprocessors: List[Text],
         train_nodes: Dict[Text, SchemaNode],
     ) -> Dict[Text, SchemaNode]:
+
         predict_config = copy.deepcopy(config)
         predict_nodes = {}
 

rasa/engine/storage/local_model_storage.py~L221

 
     @staticmethod
     def _persist_metadata(metadata: ModelMetadata, temporary_directory: Path) -> None:
+
         rasa.shared.utils.io.dump_obj_as_json_to_file(
             temporary_directory / MODEL_ARCHIVE_METADATA_FILE, metadata.as_dict()
         )

rasa/engine/validation.py~L485

     parent_return_type: TypeAnnotation,
     required_type: TypeAnnotation,
 ) -> None:
+
     if not typing_utils.issubtype(parent_return_type, required_type):
         parent_node_text = ""
         if parent_node:

rasa/nlu/classifiers/diet_classifier.py~L506

     def _extract_features(
         self, message: Message, attribute: Text
     ) -> Dict[Text, Union[scipy.sparse.spmatrix, np.ndarray]]:
+
         (
             sparse_sequence_features,
             sparse_sentence_features,

rasa/nlu/classifiers/diet_classifier.py~L775

         sparse_feature_sizes: Dict[Text, Dict[Text, List[int]]],
         label_attribute: Optional[Text] = None,
     ) -> Dict[Text, Dict[Text, List[int]]]:
+
         if label_attribute in sparse_feature_sizes:
             del sparse_feature_sizes[label_attribute]
         return sparse_feature_sizes

rasa/nlu/classifiers/diet_classifier.py~L1260

         config: Dict[Text, Any],
         finetune_mode: bool,
     ) -> "RasaModel":
+
         predict_data_example = RasaModelData(
             label_key=model_data_example.label_key,
             data={

rasa/nlu/classifiers/diet_classifier.py~L1503

         sequence_feature_lengths: tf.Tensor,
         name: Text,
     ) -> tf.Tensor:
+
         x, _ = self._tf_layers[f"feature_combining_layer.{name}"](
             (sequence_features, sentence_features, sequence_feature_lengths),
             training=self._training,

rasa/nlu/classifiers/diet_classifier.py~L1684

         return loss
 
     def _update_label_metrics(self, loss: tf.Tensor, acc: tf.Tensor) -> None:
+
         self.intent_loss.update_state(loss)
         self.intent_acc.update_state(acc)
 

rasa/nlu/classifiers/diet_classifier.py~L1842

         combined_sequence_sentence_feature_lengths: tf.Tensor,
         text_transformed: tf.Tensor,
     ) -> Dict[Text, tf.Tensor]:
+
         if self.all_labels_embed is None:
             raise ValueError(
                 "The model was not prepared for prediction. "

rasa/nlu/featurizers/dense_featurizer/convert_featurizer.py~L323

         return texts
 
     def _sentence_encoding_of_text(self, batch: List[Text]) -> np.ndarray:
+
         return self.sentence_encoding_signature(tf.convert_to_tensor(batch))[
             "default"
         ].numpy()
 
     def _sequence_encoding_of_text(self, batch: List[Text]) -> np.ndarray:
+
         return self.sequence_encoding_signature(tf.convert_to_tensor(batch))[
             "sequence_encoding"
         ].numpy()

rasa/nlu/featurizers/dense_featurizer/convert_featurizer.py~L407

             )
 
     def _tokenize(self, sentence: Text) -> Any:
+
         return self.tokenize_signature(tf.convert_to_tensor([sentence]))[
             "default"
         ].numpy()

rasa/nlu/featurizers/sparse_featurizer/count_vectors_featurizer.py~L98

         return ["sklearn"]
 
     def _load_count_vect_params(self) -> None:
+
         # Use shared vocabulary between text and all other attributes of Message
         self.use_shared_vocab = self._config["use_shared_vocab"]
 

rasa/nlu/persistor.py~L95

 
     @staticmethod
     def _tar_name(model_name: Text, include_extension: bool = True) -> Text:
+
         ext = ".tar.gz" if include_extension else ""
         return f"{model_name}{ext}"
 

rasa/nlu/selectors/response_selector.py~L625

         config: Dict[Text, Any],
         finetune_mode: bool = False,
     ) -> "RasaModel":
+
         predict_data_example = RasaModelData(
             label_key=model_data_example.label_key,
             data={

rasa/nlu/selectors/response_selector.py~L721

             logger.debug(f"  {metric} ({name})")
 
     def _update_label_metrics(self, loss: tf.Tensor, acc: tf.Tensor) -> None:
+
         self.response_loss.update_state(loss)
         self.response_acc.update_state(acc)
 

rasa/nlu/test.py~L1569

 
 
 def _contains_entity_labels(entity_results: List[EntityEvaluationResult]) -> bool:
+
     for result in entity_results:
         if result.entity_targets or result.entity_predictions:
             return True

rasa/server.py~L234

         async def decorated(
             request: Request, *args: Any, **kwargs: Any
         ) -> response.HTTPResponse:
+
             provided = request.args.get("token", None)
 
             # noinspection PyProtectedMember

rasa/shared/core/events.py~L318

     def from_parameters(
         parameters: Dict[Text, Any], default: Optional[Type["Event"]] = None
     ) -> Optional["Event"]:
+
         event_name = parameters.get("event")
         if event_name is None:
             return None

rasa/shared/core/events.py~L1019

     def _from_story_string(
         cls, parameters: Dict[Text, Any]
     ) -> Optional[List["SlotSet"]]:
+
         slots = []
         for slot_key, slot_val in parameters.items():
             slots.append(SlotSet(slot_key, slot_val))

rasa/shared/core/events.py~L1218

     def _from_story_string(
         cls, parameters: Dict[Text, Any]
     ) -> Optional[List["ReminderScheduled"]]:
+
         trigger_date_time = parser.parse(parameters.get("date_time"))
 
         return [

rasa/shared/core/events.py~L1471

     def _from_story_string(
         cls, parameters: Dict[Text, Any]
     ) -> Optional[List["FollowupAction"]]:
+
         return [
             FollowupAction(
                 parameters.get("name"),

rasa/shared/core/training_data/story_reader/story_reader.py~L111

     def _add_checkpoint(
         self, name: Text, conditions: Optional[Dict[Text, Any]]
     ) -> None:
+
         # Ensure story part already has a name
         if not self.current_step_builder:
             raise StoryParseError(

rasa/shared/core/training_data/story_reader/yaml_story_reader.py~L539

         return default_value
 
     def _parse_action(self, step: Dict[Text, Any]) -> None:
+
         action_name = step.get(KEY_ACTION, "")
         if not action_name:
             rasa.shared.utils.io.raise_warning(

rasa/shared/core/training_data/story_reader/yaml_story_reader.py~L560

         self._add_event(ActiveLoop.type_name, {LOOP_NAME: active_loop_name})
 
     def _parse_checkpoint(self, step: Dict[Text, Any]) -> None:
+
         checkpoint_name = step.get(KEY_CHECKPOINT, "")
         slots = step.get(KEY_CHECKPOINT_SLOTS, [])
 

rasa/shared/importers/importer.py~L475

         return original.merge(e2e_domain)
 
     def _get_domain_with_e2e_actions(self) -> Domain:
+
         stories = self.get_stories()
 
         additional_e2e_action_names = set()

rasa/shared/importers/rasa.py~L26

         domain_path: Optional[Text] = None,
         training_data_paths: Optional[Union[List[Text], Text]] = None,
     ):
+
         self._domain_path = domain_path
 
         self._nlu_files = rasa.shared.data.get_data_files(

rasa/shared/nlu/training_data/formats/rasa_yaml.py~L105

         )
 
     def _parse_nlu(self, nlu_data: Optional[List[Dict[Text, Any]]]) -> None:
+
         if not nlu_data:
             return
 

rasa/shared/nlu/training_data/training_data.py~L47

         lookup_tables: Optional[List[Dict[Text, Any]]] = None,
         responses: Optional[Dict[Text, List[Dict[Text, Any]]]] = None,
     ) -> None:
+
         if training_examples:
             self.training_examples = self.sanitize_examples(training_examples)
         else:

rasa/utils/tensorflow/models.py~L889

         tag_name: Text,
         entity_tags: Optional[tf.Tensor] = None,
     ) -> Tuple[tf.Tensor, tf.Tensor, tf.Tensor]:
+
         tag_ids = tf.cast(tag_ids[:, :, 0], tf.int32)
 
         if entity_tags is not None:

tests/cli/test_rasa_data.py~L50

 
 
 def test_data_convert_nlu_json(run_in_simple_project: Callable[..., RunResult]):
+
     result = run_in_simple_project(
         "data",
         "convert",

tests/cli/test_rasa_data.py~L69

 def test_data_convert_nlu_yml(
     run: Callable[..., RunResult], tmp_path: Path, request: FixtureRequest
 ):
+
     target_file = tmp_path / "out.yml"
 
     # The request rootdir is required as the `testdir` fixture in `run` changes the

tests/cli/test_rasa_train.py~L119

 def test_train_no_domain_exists(
     run_in_simple_project: Callable[..., RunResult], tmp_path: Path
 ) -> None:
+
     os.remove("domain.yml")
     run_in_simple_project(
         "train",

tests/cli/test_rasa_x.py~L146

 
 
 def test_rasa_x_raises_warning_and_exits_without_production_flag():
+
     args = argparse.Namespace(loglevel=None, log_file=None, production=None)
     with pytest.raises(SystemExit):
         with pytest.warns(

tests/core/channels/test_hangouts.py~L9

 
 
 def test_hangouts_channel():
+
     from rasa.core.channels.hangouts import HangoutsInput
     import rasa.core
 

tests/core/channels/test_hangouts.py~L161

 
 @pytest.mark.asyncio
 async def test_hangouts_output_channel_functions():
+
     from rasa.core.channels.hangouts import HangoutsOutput
 
     output_channel = HangoutsOutput()

tests/core/channels/test_twilio_voice.py~L15

 
 
 async def test_twilio_voice_twiml_response_text():
+
     inputs = {
         "initial_prompt": "hello",
         "reprompt_fallback_phrase": "i didn't get that",

tests/core/channels/test_twilio_voice.py~L43

 
 
 async def test_twilio_voice_twiml_response_buttons():
+
     inputs = {
         "initial_prompt": "hello",
         "reprompt_fallback_phrase": "i didn't get that",

tests/core/channels/test_twilio_voice.py~L156

 
 
 async def test_twilio_voice_remove_image():
+
     with pytest.warns(UserWarning):
         output_channel = TwilioVoiceCollectingOutputChannel()
         await output_channel.send_response(

tests/core/channels/test_twilio_voice.py~L165

 
 
 async def test_twilio_voice_keep_image_text():
+
     output_channel = TwilioVoiceCollectingOutputChannel()
     await output_channel.send_response(
         recipient_id="Chuck Norris",

tests/core/channels/test_twilio_voice.py~L175

 
 
 async def test_twilio_emoji_warning():
+
     with pytest.warns(UserWarning):
         output_channel = TwilioVoiceCollectingOutputChannel()
         await output_channel.send_response(

tests/core/channels/test_twilio_voice.py~L183

 
 
 async def test_twilio_voice_multiple_responses():
+
     inputs = {
         "initial_prompt": "hello",
         "reprompt_fallback_phrase": "i didn't get that",

tests/core/evaluation/test_marker.py~L651

     ],
 )
 def test_marker_from_path_adds_special_or_marker(tmp_path: Path, configs: Any):
+
     yaml_file = tmp_path / "config.yml"
     rasa.shared.utils.io.write_yaml(data=configs, target=yaml_file)
     loaded = Marker.from_path(tmp_path)

tests/core/evaluation/test_marker_stats.py~L258

 
 @pytest.mark.parametrize("seed", [2345, 5654, 2345234])
 def test_per_session_statistics_to_csv(tmp_path: Path, seed: int):
+
     rng = np.random.default_rng(seed=seed)
     (
         per_session_results,

tests/core/featurizers/test_precomputation.py~L497

     collector: CoreFeaturizationCollector,
     messages_with_unique_lookup_key: List[Message],
 ):
+
     messages = messages_with_unique_lookup_key
 
     # pass as training data

tests/core/featurizers/test_single_state_featurizers.py~L414

 
 
 def test_encode_entities__with_entity_roles_and_groups():
+
     # create fake message that has been tokenized and entities have been extracted
     text = "I am flying from London to Paris"
     tokens = [

tests/core/featurizers/test_single_state_featurizers.py~L471

 
 
 def test_encode_entities__with_bilou_entity_roles_and_groups():
+
     # Instantiate domain and configure the single state featurizer for this domain.
     # Note that there are 2 entity tags here.
     entity_tags = ["city", f"city{ENTITY_LABEL_SEPARATOR}to"]

tests/core/policies/test_ted_policy.py~L101

 class TestTEDPolicy(PolicyTestCollection):
     @staticmethod
     def _policy_class_to_test() -> Type[TEDPolicy]:
+
         return TEDPolicy
 
     def test_train_model_checkpointing(

tests/core/policies/test_unexpected_intent_policy.py~L56

 class TestUnexpecTEDIntentPolicy(TestTEDPolicy):
     @staticmethod
     def _policy_class_to_test() -> Type[UnexpecTEDIntentPolicy]:
+
         return UnexpecTEDIntentPolicy
 
     @pytest.fixture(scope="class")

tests/core/policies/test_unexpected_intent_policy.py~L98

     def test_label_data_assembly(
         self, trained_policy: UnexpecTEDIntentPolicy, default_domain: Domain
     ):
+
         # Construct input data
         state_featurizer = trained_policy.featurizer.state_featurizer
         encoded_all_labels = state_featurizer.encode_all_labels(

tests/core/policies/test_unexpected_intent_policy.py~L846

             all_similarities: np.array,
             label_index: int,
         ):
+
             expected_score = all_similarities[0][label_index]
             expected_threshold = (
                 all_thresholds[label_index] if label_index in all_thresholds else None

tests/core/test_actions.py~L787

 
 
 async def test_response_channel_specific(default_nlg, default_tracker, domain: Domain):
+
     output_channel = SlackBot("DummyToken", "General")
 
     events = await ActionBotResponse("utter_channel").run(

tests/core/test_broker.py~L99

 
 
 async def test_pika_raise_connection_exception(monkeypatch: MonkeyPatch):
+
     monkeypatch.setattr(
         PikaEventBroker, "connect", AsyncMock(side_effect=ChannelNotFoundEntity())
     )

tests/core/test_broker.py~L324

 
 
 async def test_create_pika_invalid_port():
+
     cfg = EndpointConfig(
         username="username", password="password", type="pika", port="PORT"
     )

tests/core/test_nlg.py~L12

 
 
 def nlg_app(base_url="/"):
+
     app = Sanic("test_nlg")
 
     @app.route(base_url, methods=["POST"])

tests/core/test_policies.py~L424

         default_domain: Domain,
         stories_path: Text,
     ):
+
         execution_context = dataclasses.replace(execution_context, is_finetuning=True)
         loaded_policy = MemoizationPolicy.load(
             trained_policy.config, model_storage, resource, execution_context

tests/core/test_test.py~L198

     monkeypatch: MonkeyPatch,
     moodbot_domain_path: Path,
 ) -> Callable[[Path, bool], Coroutine]:
+
     # We need `RulePolicy` to predict the correct actions
     # in a particular conversation context as seen during training.
     # Since it can get affected by `action_unlikely_intent` being triggered in

tests/core/test_tracker_stores.py~L177

 
 
 def test_redis_tracker_store_invalid_key_prefix(domain: Domain):
+
     test_invalid_key_prefix = "$$ &!"
 
     tracker_store = RedisTrackerStore(

tests/engine/recipes/test_default_recipe.py~L593

 
 
 def test_dump_config_missing_file(tmp_path: Path, capsys: CaptureFixture):
+
     config_path = tmp_path / "non_existent_config.yml"
 
     config = rasa.shared.utils.io.read_config_file(str(SOME_CONFIG))

tests/engine/test_caching.py~L50

         model_storage: ModelStorage,
         output_fingerprint: Text,
     ) -> "TestCacheableOutput":
+
         value = rasa.shared.utils.io.read_json_file(directory / "cached.json")
 
         return cls(value, cache_dir=directory)

tests/engine/test_validation.py~L91

     language: Optional[Text] = None,
     is_train_graph: bool = True,
 ) -> GraphModelConfiguration:
+
     parent_node = {}
     if parent:
         parent_node = {

tests/engine/training/test_components.py~L17

 
 
 def test_cached_component_returns_value_from_cache(default_model_storage: ModelStorage):
+
     cached_output = CacheableText("Cache me!!")
 
     node = GraphNode(

tests/engine/training/test_components.py~L105

 def test_fingerprint_component_hit(
     default_model_storage: ModelStorage, temp_cache: TrainingCache
 ):
+
     cached_output = CacheableText("Cache me!!")
     output_fingerprint = uuid.uuid4().hex
 

tests/engine/training/test_components.py~L159

 def test_fingerprint_component_miss(
     default_model_storage: ModelStorage, temp_cache: TrainingCache
 ):
+
     component_config = {"x": 1}
 
     node = GraphNode(

tests/engine/training/test_graph_trainer.py~L108

     train_with_schema: Callable,
     spy_on_all_components: Callable,
 ):
+
     input_file = tmp_path / "input_file.txt"
     input_file.write_text("3")
 

tests/engine/training/test_graph_trainer.py~L206

     train_with_schema: Callable,
     spy_on_all_components: Callable,
 ):
+
     input_file = tmp_path / "input_file.txt"
     input_file.write_text("3")
 

tests/engine/training/test_graph_trainer.py~L259

     train_with_schema: Callable,
     spy_on_all_components: Callable,
 ):
+
     input_file = tmp_path / "input_file.txt"
     input_file.write_text("3")
 

tests/engine/training/test_graph_trainer.py~L372

     train_with_schema: Callable,
     caplog: LogCaptureFixture,
 ):
+
     input_file = tmp_path / "input_file.txt"
     input_file.write_text("3")
 

tests/examples/test_example_bots_training_data.py~L65

     raise_slot_warning: bool,
     msg: Optional[Text],
 ):
+
     importer = TrainingDataImporter.load_from_config(
         config_file, domain_file, [data_folder]
     )

tests/graph_components/validators/test_default_recipe_validator.py~L685

 def test_core_warn_if_data_but_no_policy(
     monkeypatch: MonkeyPatch, policy_type: Optional[Type[Policy]]
 ):
+
     importer = TrainingDataImporter.load_from_dict(
         domain_path="data/test_e2ebot/domain.yml",
         training_data_paths=[

tests/graph_components/validators/test_default_recipe_validator.py~L814

 def test_core_raise_if_a_rule_policy_is_incompatible_with_domain(
     monkeypatch: MonkeyPatch,
 ):
+
     domain = Domain.empty()
 
     num_instances = 2

tests/graph_components/validators/test_default_recipe_validator.py~L859

     num_duplicates: bool,
     priority: int,
 ):
+
     assert len(policy_types) >= priority + num_duplicates, (
         f"This tests needs at least {priority + num_duplicates} many types."
     )

tests/graph_components/validators/test_default_recipe_validator.py~L923

 
 @pytest.mark.parametrize("policy_type_consuming_rule_data", [RulePolicy])
 def test_core_warn_if_rule_data_missing(policy_type_consuming_rule_data: Type[Policy]):
+
     importer = TrainingDataImporter.load_from_dict(
         domain_path="data/test_e2ebot/domain.yml",
         training_data_paths=[

tests/graph_components/validators/test_default_recipe_validator.py~L952

 def test_core_warn_if_rule_data_unused(
     policy_type_not_consuming_rule_data: Type[Policy],
 ):
+
     importer = TrainingDataImporter.load_from_dict(
         domain_path="data/test_moodbot/domain.yml",
         training_data_paths=[

tests/nlu/classifiers/test_diet_classifier.py~L132

         message_text: Text = "Rasa is great!",
         expect_intent: bool = True,
     ) -> Message:
+
         if not pipeline:
             pipeline = [
                 {"component": WhitespaceTokenizer},

tests/nlu/classifiers/test_regex_message_handler.py~L38

 def test_process_does_not_do_anything(
     regex_message_handler: RegexMessageHandler, text: Text
 ):
+
     message = Message(
         data={TEXT: text, INTENT: "bla"},
         features=[

tests/nlu/classifiers/test_regex_message_handler.py~L77

 def test_regex_message_handler_adds_extractor_name(
     regex_message_handler: RegexMessageHandler, text: Text
 ):
+
     message = Message(
         data={TEXT: text, INTENT: "bla"},
         features=[

tests/nlu/conftest.py~L39

     def inner(
         pipeline: List[Dict[Text, Any]], training_data: Union[Text, TrainingData]
     ) -> Tuple[TrainingData, List[GraphComponent]]:
+
         if isinstance(training_data, str):
             importer = RasaFileImporter(training_data_paths=[training_data])
             training_data: TrainingData = importer.get_nlu_data()

tests/nlu/conftest.py~L75

 @pytest.fixture()
 def process_message(default_model_storage: ModelStorage) -> Callable[..., Message]:
     def inner(loaded_pipeline: List[GraphComponent], message: Message) -> Message:
+
         for component in loaded_pipeline:
             component.process([message])
 

tests/nlu/extractors/test_crf_entity_extractor.py~L128

     spacy_nlp_component: SpacyNLP,
     spacy_model: SpacyModel,
 ):
+
     crf_extractor = crf_entity_extractor(config_params)
 
     importer = RasaFileImporter(training_data_paths=["data/examples/rasa"])

tests/nlu/extractors/test_mitie_entity_extractor.py~L73

     mitie_model: MitieModel,
     with_trainable_examples: bool,
 ):
+
     # some texts where last token is a city
     texts_ending_with_city = ["Bert lives in Berlin", "Ernie asks where is Bielefeld"]
 

tests/nlu/extractors/test_regex_entity_extractor.py~L278

 def test_process_does_not_overwrite_any_entities(
     create_or_load_extractor: Callable[..., RegexEntityExtractor],
 ):
+
     pre_existing_entity = {
         ENTITY_ATTRIBUTE_TYPE: "person",
         ENTITY_ATTRIBUTE_VALUE: "Max",

tests/nlu/featurizers/test_lexical_syntactic_featurizer.py~L239

     resource_lexical_syntactic_featurizer: Resource,
     feature_config: List[Text],
 ) -> Callable[..., LexicalSyntacticFeaturizer]:
+
     config = {"alias": "lsf", "features": feature_config}
     featurizer = create_lexical_syntactic_featurizer(config)
 

tests/nlu/featurizers/test_lexical_syntactic_featurizer.py~L300

     feature_config: Dict[Text, Any],
     expected_features: np.ndarray,
 ):
+
     featurizer = create_lexical_syntactic_featurizer({
         "alias": "lsf",
         "features": feature_config,

tests/nlu/featurizers/test_lm_featurizer.py~L810

         [Dict[Text, Any]], LanguageModelFeaturizer
     ],
 ):
+
     config = {"model_name": "bert", "model_weights": "bert-base-chinese"}
 
     lm_featurizer = create_language_model_featurizer(config)

tests/nlu/featurizers/test_mitie_featurizer.py~L57

     mitie_model: MitieModel,
     mitie_tokenizer: MitieTokenizer,
 ):
+
     featurizer = create({"alias": "mitie_featurizer"})
 
     sentence = "Hey how are you today"

tests/nlu/featurizers/test_mitie_featurizer.py~L87

     mitie_model: MitieModel,
     mitie_tokenizer: MitieTokenizer,
 ):
+
     featurizer = create({"alias": "mitie_featurizer"})
 
     sentence = "Hey how are you today"

tests/nlu/featurizers/test_regex_featurizer.py~L302

     create_featurizer: Callable[..., RegexFeaturizer],
     spacy_tokenizer: SpacyTokenizer,
 ):
+
     patterns = [
         {"pattern": "[0-9]+", "name": "number", "usage": "intent"},
         {"pattern": "\\bhey*", "name": "hello", "usage": "intent"},

tests/nlu/featurizers/test_regex_featurizer.py~L398

     create_featurizer: Callable[..., RegexFeaturizer],
     spacy_tokenizer: SpacyTokenizer,
 ):
+
     patterns = [
         {"pattern": "[0-9]+", "name": "number", "usage": "intent"},
         {"pattern": "\\bhey*", "name": "hello", "usage": "intent"},

tests/nlu/featurizers/test_spacy_featurizer.py~L39

 
 @pytest.mark.parametrize("sentence", ["hey how are you today"])
 def test_spacy_featurizer(sentence, spacy_nlp):
+
     ftr = create_spacy_featurizer({})
 
     doc = spacy_nlp(sentence)

tests/nlu/featurizers/test_spacy_featurizer.py~L157

 
 
 def test_spacy_featurizer_train(spacy_nlp):
+
     featurizer = create_spacy_featurizer({})
 
     sentence = "Hey how are you today"

tests/nlu/selectors/test_selectors.py~L125

         config_params: Dict[Text, Any],
         should_finetune: bool,
     ):
+
         training_data, loaded_pipeline = train_and_preprocess(
             pipeline, "data/examples/rasa/demo-rasa.yml"
         )

tests/nlu/selectors/test_selectors.py~L225

     response_selector_training_data: TrainingData,
     create_response_selector: Callable[[Dict[Text, Any]], ResponseSelector],
 ):
+
     training_data_extra_intent = TrainingData([
         Message.build(text="Is it possible to detect the version?", intent="faq/q1"),
         Message.build(text="How can I get a new virtual env", intent="faq/q2"),

tests/nlu/selectors/test_selectors.py~L276

     response_selector_training_data: TrainingData,
     create_response_selector: Callable[[Dict[Text, Any]], ResponseSelector],
 ):
+
     response_selector = create_response_selector({"use_text_as_label": train_on_text})
     response_selector.preprocess_train_data(response_selector_training_data)
 

tests/nlu/selectors/test_selectors.py~L342

     default_execution_context: ExecutionContext,
     train_persist_load_with_different_settings,
 ):
+
     pipeline = [
         {"component": WhitespaceTokenizer},
         {"component": CountVectorsFeaturizer},

tests/nlu/test_evaluation.py~L918

 
 
 def test_entity_evaluation_report(tmp_path: Path):
+
     path = tmp_path / "evaluation"
     path.mkdir()
     report_folder = str(path / "reports")

tests/nlu/tokenizers/test_whitespace_tokenizer.py~L80

     ],
 )
 def test_whitespace(text, expected_tokens, expected_indices):
+
     tk = create_whitespace_tokenizer()
 
     tokens = tk.tokenize(Message.build(text=text), attribute=TEXT)

tests/nlu/utils/test_bilou_utils.py~L88

 
 
 def test_apply_bilou_schema(whitespace_tokenizer: WhitespaceTokenizer):
+
     message_1 = Message.build(
         text="Germany is part of the European Union", intent="inform"
     )

tests/nlu/utils/test_bilou_utils.py~L228

     debug_message: Optional[Text],
     caplog: LogCaptureFixture,
 ):
+
     with caplog.at_level(logging.DEBUG):
         actual_tags, actual_confidences = bilou_utils.ensure_consistent_bilou_tagging(
             tags, confidences

tests/shared/core/training_data/story_reader/test_yaml_story_reader.py~L300

 
 
 def test_read_rules_with_stories(domain: Domain):
+
     yaml_file = "data/test_yaml_stories/stories_and_rules.yml"
 
     steps = loading.load_data_from_files([yaml_file], domain)

tests/shared/nlu/training_data/formats/test_rasa_yaml.py~L132

 
 
 def test_wrong_format_raises():
+
     wrong_yaml_nlu_content = """
     !!
     """

tests/shared/nlu/training_data/formats/test_rasa_yaml.py~L145

     "example", [WRONG_YAML_NLU_CONTENT_1, WRONG_YAML_NLU_CONTENT_2]
 )
 def test_wrong_schema_raises(example: Text):
+
     parser = RasaYAMLReader()
     with pytest.raises(YamlException):
         parser.reads(example)

tests/shared/nlu/training_data/formats/test_rasa_yaml.py~L336

 
 
 def test_lookup_is_parsed():
+
     parser = RasaYAMLReader()
     training_data = parser.reads(LOOKUP_EXAMPLE)
 

tests/shared/nlu/training_data/formats/test_rasa_yaml.py~L344

 
 
 def test_regex_is_parsed():
+
     parser = RasaYAMLReader()
     training_data = parser.reads(REGEX_EXAMPLE)
 

tests/shared/nlu/training_data/test_entities_parser.py~L131

 def test_markdown_entity_regex(
     example: Text, expected_entities: List[Dict[Text, Any]], expected_text: Text
 ):
+
     result = entities_parser.find_entities_in_training_example(example)
     assert result == expected_entities
 

tests/shared/nlu/training_data/test_features.py~L250

     ),
 )
 def test_combine(is_sparse: bool, type: Text, number: int, use_expected_origin: bool):
+
     features_list, modifications = _generate_feature_list_and_modifications(
         is_sparse=is_sparse, type=type, number=number
     )

tests/shared/nlu/training_data/test_features.py~L291

     ),
 )
 def test_filter(is_sparse: bool, type: Text, number: int):
+
     features_list, modifications = _generate_feature_list_and_modifications(
         is_sparse=is_sparse, type=type, number=number
     )

tests/shared/nlu/training_data/test_features.py~L344

     num_features_per_attribute: Dict[Text, int],
     specified_attributes: Optional[List[Text]],
 ):
+
     features_list = []
     for attribute, number in num_features_per_attribute.items():
         for idx in range(number):

tests/shared/nlu/training_data/test_features.py~L382

 def test_reduce(
     shuffle_mode: Text, num_features_per_combination: Tuple[int, int, int, int]
 ):
+
     # all combinations - in the expected order
     # (i.e. all sparse before all dense and sequence before sentence)
     all_combinations = [

tests/shared/nlu/training_data/test_message.py~L60

     expected_seq_features: Optional[List[Features]],
     expected_sen_features: Optional[List[Features]],
 ):
+
     message = Message(data={TEXT: "This is a test sentence."}, features=features)
 
     actual_seq_features, actual_sen_features = message.get_dense_features(

tests/shared/nlu/training_data/test_synonyms_parser.py~L2

 
 
 def test_add_synonym():
+
     synonym_name = "savings"
     synonym_examples = ["pink pig", "savings account"]
     expected_result = {"pink pig": synonym_name, "savings account": synonym_name}

tests/shared/nlu/training_data/test_synonyms_parser.py~L15

 
 
 def test_add_synonyms_from_entities():
+
     training_example = "I want to fly from Berlin to LA"
 
     entities = [

tests/shared/utils/schemas/test_events.py~L72

 
 @pytest.mark.parametrize("event_class", rasa.shared.utils.common.all_subclasses(Event))
 def test_remote_action_validate_all_event_subclasses(event_class: Type[Event]):
+
     if event_class.type_name == "slot":
         response = {
             "events": [{"event": "slot", "name": "test", "value": "example"}],

tests/shared/utils/test_io.py~L591

 
 @pytest.mark.parametrize("length", [4, 8, 16, 32])
 def test_random_string(length):
+
     s = rasa.shared.utils.io.random_string(length)
     s2 = rasa.shared.utils.io.random_string(length)
 

tests/shared/utils/test_validation.py~L295

 
 
 async def test_future_training_data_format_version_not_compatible():
+
     next_minor = str(Version(LATEST_TRAINING_DATA_FORMAT_VERSION).next_minor())
 
     incompatible_version = {KEY_TRAINING_DATA_FORMAT_VERSION: next_minor}

tests/shared/utils/test_validation.py~L306

 
 
 async def test_compatible_training_data_format_version():
+
     prev_major = str(Version("1.0"))
 
     compatible_version_1 = {KEY_TRAINING_DATA_FORMAT_VERSION: prev_major}

tests/shared/utils/test_validation.py~L319

 
 
 async def test_invalid_training_data_format_version_warns():
+
     invalid_version_1 = {KEY_TRAINING_DATA_FORMAT_VERSION: 2.0}
     invalid_version_2 = {KEY_TRAINING_DATA_FORMAT_VERSION: "Rasa"}
 

tests/test_server.py~L693

 
 
 async def test_add_message(rasa_app: SanicASGITestClient):
+
     conversation_id = "test_add_message_test_id"
 
     _, response = await rasa_app.get(f"/conversations/{conversation_id}/tracker")

tests/utils/tensorflow/test_models.py~L35

     new_batch_outputs: Dict[Text, Union[np.ndarray, Dict[Text, np.ndarray]]],
     expected_output: Dict[Text, Union[np.ndarray, Dict[Text, np.ndarray]]],
 ):
+
     predicted_output = RasaModel._merge_batch_outputs(
         existing_outputs, new_batch_outputs
     )

tests/utils/tensorflow/test_models.py~L72

     def _batch_predict(
         batch_in: Tuple[np.ndarray],
     ) -> Dict[Text, Union[np.ndarray, Dict[Text, np.ndarray]]]:
+
         dummy_output = batch_in[0]
         output = {
             "dummy_output": dummy_output,

Snowflake-Labs/snowcli (+59 -0 lines across 34 files)

ruff format --preview

src/snowflake/cli/_plugins/logs/manager.py~L87

         log_level: Optional[str] = "INFO",
         partial_match: bool = False,
     ) -> SnowflakeCursor:
+
         table = event_table if event_table else "SNOWFLAKE.TELEMETRY.EVENTS"
 
         # Escape single quotes in object_name to prevent SQL injection

src/snowflake/cli/_plugins/snowpark/snowpark_entity.py~L41

 
 class SnowparkEntity(EntityBase[Generic[T]]):
     def __init__(self, *args, **kwargs):
+
         if not FeatureFlag.ENABLE_NATIVE_APP_CHILDREN.is_enabled():
             raise NotImplementedError("Snowpark entity is not implemented yet")
         super().__init__(*args, **kwargs)

src/snowflake/cli/_plugins/snowpark/zipper.py~L67

     dest_zip: Path,
     mode: Literal["r", "w", "x", "a"] = "w",
 ) -> None:
+
     if not dest_zip.parent.exists():
         SecurePath(dest_zip).parent.mkdir(parents=True)
 

src/snowflake/cli/_plugins/spcs/compute_pool/manager.py~L42

         comment: Optional[str],
         if_not_exists: bool,
     ) -> SnowflakeCursor:
+
         create_statement = "CREATE COMPUTE POOL"
         if if_not_exists:
             create_statement = f"{create_statement} IF NOT EXISTS"

src/snowflake/cli/_plugins/spcs/compute_pool/manager.py~L79

         comment: Optional[str],
  ...*[Comment body truncated]*

[ntBre]

The ecosystem results look good to me. There were only a couple of somewhat surprising cases:

• https://github.com/RasaHQ/rasa/blob/b8de3b231126747ff74b2782cb25cb22d2d898d7/tests/nlu/conftest.py#L79

     @pytest.fixture()
    def process_message(default_model_storage: ModelStorage) -> Callable[..., Message]:
        def inner(loaded_pipeline: List[GraphComponent], message: Message) -> Message:
    
            for component in loaded_pipeline:
                component.process([message])
    
            return message
    
        return inner

• https://github.com/snowflakedb/snowflake-cli/blob/b0306e131063d26be11f14cb1f9dbd9ae67b32c8/src/snowflake/cli/_plugins/snowpark/snowpark_entity.py#L44

    class SnowparkEntity(EntityBase[Generic[T]]):
        def __init__(self, *args, **kwargs):
    
            if not FeatureFlag.ENABLE_NATIVE_APP_CHILDREN.is_enabled():
                raise NotImplementedError("Snowpark entity is not implemented yet")
            super().__init__(*args, **kwargs)

These both feel a bit awkward to me because they preserve a blank line inside the inner functions but not outside. But I guess we're only preserving the input whitespace, so this might still be what the authors intended.

[MichaReiser]

I like the idea of restricting it to functions with docstrings but I'm also still concerned that it reduces consistency and will lead to formatting discussions on diff, the very problem Ruff format tries to solve.

For example, I dislike this change and I'd be inclined to comment in the PR review that the empty line here is unnecessary:

 def nlg_app(base_url="/"):
+
     app = Sanic("test_nlg")
 
     @app.route(base_url, methods=["POST"])

Having said that, I think this strikes a good balance.

I went through the original issue and I noticed that we don't preserve the empty line for

if long_boolean_variable_name and another_long_conditional:

    # A pretty long comment documenting what the block does
    print("Do stuff")

Which I'd find surprising and seems worth checking if we can support it too

[MichaReiser]

Can we add more tests to this, specifically tests including comments or cases where the first element is a function on its own?

[MichaReiser]

This elipsis check should exactly match the check we do in the function formatting (also test for trailing comments)

ruff/crates/ruff_python_formatter/src/statement/clause.rs

Lines 451 to 453 in c4506f0

	if should_collapse_stub
	&& contains_only_an_ellipsis(self.body, f.context().comments())
	&& self.trailing_comments.is_empty()

[ntBre]

I added two new tests trying to exercise this, and they both seemed to work as I expected without any additional check:

def preserved5():

    ...
    # trailing comment prevents collapsing the stub


def removed4():

    ...  # trailing same-line comment does not prevent collapsing the stub

and in fact when I updated this condition to:

            let will_collapse_stub_function =
                contains_only_an_ellipsis(statements, f.context().comments())
                    && !first_comments.has_trailing();

I caused another syntax error for the removed4 case. contains_only_an_ellipsis already has one kind of check for trailing comments:

ruff/crates/ruff_python_formatter/src/statement/suite.rs

Lines 730 to 739 in d38a529

	pub(crate) fn contains_only_an_ellipsis(body: &[Stmt], comments: &Comments) -> bool {
	match body {
	[Stmt::Expr(ast::StmtExpr { value, .. })] => {
	let [node] = body else {
	return false;
	};
	value.is_ellipsis_literal_expr()
	&& !comments.has_leading(node)
	&& !comments.has_trailing_own_line(node)
	}

but I'm very likely to be missing something. Is first_comments not what I should be checking?

[MichaReiser]

Maybe it's not relevant. You have to trace through FormatStmtFunctionDef.

But yes, first_comments is the wrong thing to test. trailing_comments in FormatClauseBody are the trailing_definition_comments in FormatStmtFunctionDef. So I think it's more about something like

def test(): # comment


	...

[ntBre]

Ah okay, I see now. Thanks! Yes, your example comment prevents collapsing the stub in FormatClauseBody, but we remove the newline because the logic is different.

Is there a way to pass this should_collapse_stub information into the suite formatting (maybe another with_options implementation?)? Or alternatively a way to retrieve the parent function definition so that we could recompute it?

[ntBre]

As we discussed on Discord, I added a variant of contains_only_an_ellipsis that returns the ellipsis statement, which we can then format directly in FormatClauseBody instead of calling into the suite formatting at all. Now the blank line in this example is correctly preserved.

[MichaReiser]

I think this is correct but maybe worth adding a test that we collapse any empty line before a comment:

def test():

    # comment

    a

gets formatted to

def test():
    # comment

    a

[ntBre]

Is that different from the new test here?

ruff/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py

Lines 358 to 362 in abb54b3

	def removed2():
	# Comment
	return 1

It looks like the formatting got slightly mangled in your comment.

[ntBre]

Ah I see, this is what's coming up in the ecosystem check now. I accidentally skirted around this by not preserving cases with any leading comments.

[ntBre]

lines_before may not be what I want here, if we want to preserve blank lines before comments. All of these are returning 2 lines_before:

def untouched1():

    # comment
    # more lines in the comment
    # and one more

    return 0


def untouched2():

    # comment

    return 0


def untouched3():
    # comment

    return 0

[MichaReiser]

That, or you'd have to pass the position of the first leading comment as the offset

[MichaReiser]

Note, we do have a lines_after_ignore_trivia (or similar) that uses the tokenizer over doing a string search

[ntBre]

Ah I didn't consider that I could pass a comment offset there. I assumed it had to be a statement node. Thanks!

I saw lines_after_ignore_trivia, but not a before variant. Happy to try that approach if it would be better, but using the comment offset seems to be working well.

[ntBre]

I went through the original issue and I noticed that we don't preserve the empty line for

Should we try preserving the empty line for any if, any if with a "long"/compound condition, and/or where the body starts with a comment? Just checking which part of the example to focus on.

One reason I was wary of the conditionals is that they don't seem much different from loops. For example:

for i in some_generator_function(
    with_many_arguments=[1, 2, 3], another=(4, 5, 6), one_more=None
):

    # a comment
    print("body of the loop")

Should we handle those too?

Functions seemed like a good place to draw the line to me, but I can see an argument for more consistency too.

[MichaReiser]

Should we try preserving the empty line for any if, any if with a "long"/compound condition, and/or where the body starts with a comment? Just checking which part of the example to focus on.

No sorry. I think I tested it with a function header but then copied the original example from the issue, but maybe I'm just jetlagged and tested it with a function header?

So what I think we should preserve is:

def foo():

    # A pretty long comment documenting what the block does
    print("Do stuff")

[ntBre]

Ohh okay, that makes sense! So preserve lines before comments instead of removing them. I think that does make sense.

At least one person on one of the Black issues even wanted to preserve empty lines before docstrings. I don't feel too strongly about either comments or docstrings, I was just trying to narrow the scope even further.

[MichaReiser]

At least one person on one of the Black issues even wanted to preserve empty lines before docstrings. I don't feel too strongly about either comments or docstrings, I was just trying to narrow the scope even further.

The docstrings make sense to me (not allowing an empty line) and is sort of named exactly in the rule. The comments just were a surprise to me (but I don't feel strongly about it)

[ntBre]

Oops, last commit was too simplistic. We're now adding blank lines before comments rather than just preserving them. That looks like the majority of new ecosystem hits.

[ntBre]

The ecosystem check looks good to me again. I didn't click through to every change, but I downloaded the full output from the Actions log and checked more than the subset in the comment.

This might be expected, but most of the links don't point to the exact line of the diff, which makes it a bit more of a manual process. I checked the script and this is expected.