Skip to content

Comments

Extended transformer signature#93

Open
Tishka17 wants to merge 3 commits intodevelopfrom
feature/extended_transofrmer_data
Open

Extended transformer signature#93
Tishka17 wants to merge 3 commits intodevelopfrom
feature/extended_transofrmer_data

Conversation

@Tishka17
Copy link
Member

No description provided.

@github-actions
Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/descanso
  bound_method.py 63-66, 98-101, 116
  fields.py 47
  jsonrpc.py 130, 162, 193, 198, 213, 218
  method_pipeline.py
  method_spec.py
  request.py 49
  request_transformers.py
  response.py 26, 34, 51, 94-103
  response_transformers.py
  rest_builder.py
  signature.py
Project Total  

This report was generated by python-coverage-comment-action

@sonarqubecloud
Copy link

return hints.get("return", Any)


def make_method_spec(
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should rename the method to make_method_pipeline?

)
fields_in = get_func_fields(func, is_in_class=is_in_class)
fields_out: list[FieldOut] = []
for r in transformers:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why the loop variable has the name r, and not, for example, transformer.

FieldDestination,
FieldIn,
FieldOut,
from .client import Dumper, Loader
Copy link
Contributor

Choose a reason for hiding this comment

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

are relative imports preferable?

def make_request(
client: BaseClient,
spec: MethodSpec,
spec: MethodPipeline,
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be better to rename the argument to pipeline? spec is used in many places in the project for MethodSpec it may seem that MethodSpec is enough here.

from .method_spec import MethodSpec


class FieldDestination(Enum):
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add a unique decorator for the future?

raise NotImplementedError

@abstractmethod
def transform_fields(
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we inherit the same definition from Transformer? We don't have to duplicate the definition.

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