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

Use form2request for the HttpRequest.from_form implementation #206

Merged
merged 6 commits into from
Jul 16, 2024

Conversation

Gallaecio
Copy link
Member

No description provided.

@Gallaecio Gallaecio requested review from kmike and wRAR July 12, 2024 11:51
setup.py Outdated Show resolved Hide resolved
@Gallaecio
Copy link
Member Author

@kmike What do you think about implementing a from_dataclass method (open to name suggestions) that supports dataclasses like form2request.Request?

In Formasaurus I’m thinking of using form2request directly, to not depend on web-poet, and return a form2request.Request as well, which users of both Formasaurus and web-poet will want to convert to HttpRequest.

It’s not a hard conversion to do, so users could do it manually. I just wonder whether or not it is a good idea. My main concerns are that “dataclass” is quite generic, if someone makes up some dataclass-based request data container we may not support it, but I also do not think a from_form2request_request class method is a good idea.

@kmike
Copy link
Member

kmike commented Jul 12, 2024

I don't like "from_dataclass" for the same reason - it assumes a very specific dataclass, the one returned by form2request.

What do you think about adding to_... methods to form2request?

  • to_scrapy
  • to_poet
  • to_requests

It seems it'd solve the issue, and also it'd simplify form2request docs and usage.

@Gallaecio
Copy link
Member Author

Shall we remove HttpRequest.from_form then? (has not been released yet)

@kmike
Copy link
Member

kmike commented Jul 15, 2024

Shall we remove HttpRequest.from_form then? (has not been released yet)

Hm, a good question. If we go with to.. methods, It looks redundant indeed, as HttpRequest.from_form(form) is the same as form2request(form).to_poet().

This does make this feature less discoverable for web-poet users though. So, no strong opinion on that.

@Gallaecio
Copy link
Member Author

Maybe we can address discoverability through docs.

@Gallaecio Gallaecio merged commit ce2109e into master Jul 16, 2024
19 checks passed
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