Skip to content

Conversation

fkromer
Copy link
Contributor

@fkromer fkromer commented Oct 11, 2025

No description provided.

@fkromer fkromer requested a review from izar as a code owner October 11, 2025 18:16
@fkromer
Copy link
Contributor Author

fkromer commented Oct 11, 2025

closes #287

@fkromer fkromer marked this pull request as draft October 11, 2025 18:18
@fkromer fkromer marked this pull request as ready for review October 11, 2025 18:30
@fkromer
Copy link
Contributor Author

fkromer commented Oct 12, 2025

The configuration defines defaults explicitly. Format rules are configured to work with google docstring format (has no formal spec, is de-facto standard). Single quotes are migrated to double-quotes (cause de-facto standard.)

build-backend = "poetry.masonry.api"

[tool.ruff]
target-version = "py310"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support for 3.9 will run out end of October 2025. https://devguide.python.org/versions/

@colesmj
Copy link
Collaborator

colesmj commented Oct 14, 2025

@fkromer Thank you for the requested changes,

A couple of comments on your pull request:

  1. This does not seem to be a clean PR for the intended purpose - the PR title is to add support for Ruff (a Rust-based linter for Python), yet there are multitude of other changes in your commits (mostly minor reformatting and other changes related to 3.9 end of support perhaps?). It would be ideal to have a clean PR for the addition of Ruff.

  2. Ruff is a Rust-based linter for Python, a cutting edge module that is rapidly evolving - please make the case if you can why pylint, a mature python-based linter for python, is not sufficient or desired.

  3. Thank you for the realpython link on docstrings. As a project we should consider which to standardize on; reStructuredText or NumPy/SciPy seem to be the more python-appropriate and formalized while the google docstring is simpler but as you highlighted there is not formal spec.

@fkromer
Copy link
Contributor Author

fkromer commented Oct 14, 2025

@colesmj Thanks for the review.

  1. You are right, the minor reformatting is not specific to the PR. The purpose of the commit is to temporarily showcase in what code formatting would result in. I can revert the commits right before PR merge and provide as separate PR afterwards.
  2. ruff is the de-factor standard in modern Python projects. It provides hundreds of rules out of the box and replaces a variety of other tools (black -> formatting, isort -> import sorting, mccabe -> complexity analyzation, ...) cause ruff adopted their most reasonable and valuable rules. fastapi uses ruff, pydantic uses ruff, a lot of libraries in the ML domain use it. TBH I don't know of a single modern project which is using pylint these days anymore.
  3. Right, it would make sense to agree on a docstring format still. Is not part of this PR. The current configuration has been made consistent with the google style docstrings which are in the current code base already. Changing those should be a rather simple AI driven "replace the google style docstrings with XXX style docstrings".

I'll comment in the PR for more fine-grained further explanation.


def req_reply(src: Element, dest: Element, req_name: str, reply_name=None) -> (DF, DF):
'''
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of de-facto standard. I don't know any project which is using ```. Default formatter config value.

logger-objects = []
per-file-ignores = {}
preview = false
select = ["E4", "E7", "E9", "F"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ruff rule set can be extended here. E.g. to add pylint rules refer to here.

"""
if not reply_name:
reply_name = f'Reply to {req_name}'
reply_name = f"Reply to {req_name}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

" is kind of-defacto standard (config default value).

from .pytm import (
TM,
Boundary,
Element,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code not used, to showcase code cleanup capabilities (--fix).

return result

elif spec.startswith("call:"):
# Example usage, format, exampple format
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong comment indentation fixed.

import re
import unittest
import tempfile
from contextlib import redirect_stdout
Copy link
Contributor Author

@fkromer fkromer Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code fixed (--fix).

@fkromer
Copy link
Contributor Author

fkromer commented Oct 15, 2025

Relates to #294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants