✨ Import and re-export data structures from Starlette, used by Request properties, on `fastapi.datastructures` #1872

jamescurtin · 2020-08-10T14:16:13Z

codecov · 2020-08-10T14:17:54Z

Codecov Report

Merging #1872 (3866e3d) into master (6c80e9a) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1872   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          401       401           
  Lines        10080     10095   +15     
=========================================
+ Hits         10080     10095   +15

Impacted Files	Coverage Δ
fastapi/datastructures.py	`100.00% <100.00%> (ø)`
fastapi/applications.py	`100.00% <0.00%> (ø)`
fastapi/openapi/utils.py	`100.00% <0.00%> (ø)`
docs_src/metadata/tutorial001.py	`100.00% <0.00%> (ø)`
...ts/test_tutorial/test_metadata/test_tutorial001.py	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c80e9a...3866e3d. Read the comment docs.

github-actions · 2020-08-10T15:11:21Z

📝 Docs preview for commit f5e665f at: https://5f31637f500fdecfd2a38cf2--fastapi.netlify.app

SirTelemak · 2020-08-10T16:44:34Z

Why not import it in fastapi.datastructures for more compatible syntax? Imo it's unintuitive that starlette.datastructures models imported from fastapi.requests.
Moreover in your example:

def example_get_url_from_request(request: Request) -> URL:

there URL is not request, it's response which, again, unintuitive in my opinion.

jamescurtin · 2020-08-11T12:39:29Z

Why not import it in fastapi.datastructures for more compatible syntax? Imo it's unintuitive that starlette.datastructures models imported from fastapi.requests.

Good point; restructured such that the imports mirror the structure of starlette

Kludex

My suggestions are only focused on the syntax. I'm not sure about the idea of the changes tho. I saw the issue about this, but I'm still not sure.

Kludex · 2020-08-11T12:50:21Z

fastapi/datastructures.py

@@ -1,5 +1,11 @@
 from typing import Any, Callable, Iterable, Type

+from starlette.datastructures import URL  # noqa: F401


I don't know how to create a multiline suggestion, but:

Suggested change

from starlette.datastructures import URL # noqa: F401

from starlette.datastructures import (

Address,

FormData,

Headers,

QueryParams,

State,

UploadFile as StarletteUploadFile,

URL

)

In general I would agree, but the change you suggest is incompatible with the bash scripts/format-imports.sh script.

That script invokes isort fastapi tests docs_src scripts --force-single-line-imports, which splits each import to a single line, but does not propagate the noqa hint to each line. It then invokes autoflake --remove-all-unused-imports ... which deletes the imports.

Kludex · 2020-08-11T12:50:30Z

fastapi/__init__.py

@@ -1,7 +1,6 @@
 """FastAPI framework, high performance, easy to learn, fast to code, ready for production"""

 __version__ = "0.61.0"
-


Readd this line, please

This diff was generated by running bash scripts/format-imports.sh. I just confirmed by re-adding it and running the format script; it removes the line again.

Kludex · 2020-08-11T12:52:06Z

fastapi/types.py

@@ -0,0 +1 @@
+from starlette.types import Receive  # noqa: F401


Yes--thank you! (Though it was actually the one in fastapi/requests.py that was repeated)

github-actions · 2020-08-11T13:10:44Z

📝 Docs preview for commit bf8e816 at: https://5f3298c76d5d9fb790e318dc--fastapi.netlify.app

github-actions · 2020-08-11T14:13:04Z

📝 Docs preview for commit 2202dc6 at: https://5f32a720f8bd09a98ec04244--fastapi.netlify.app

jamescurtin · 2020-08-25T00:48:43Z

👋 Checking in on this PR: would someone please be able to review? Thank you!

github-actions · 2021-07-29T15:28:43Z

📝 Docs preview for commit 3866e3d at: https://6102c8fb4d4db930370e63f7--fastapi.netlify.app

tiangolo · 2021-07-29T15:29:02Z

Great, thanks @jamescurtin for your contribution! 🚀

Thanks @Kludex and @SirTelemak for the discussion!

I updated it a bit to keep the current import style that improves typing support in editors (e.g. autoimports).

This will be available later today in FastAPI version 0.68.0 🔖🎉

…t properties, on `fastapi.datastructures` (tiangolo#1872) Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>

Import types used by Request properties

f5e665f

Make imports match starlette module structure

bf8e816

Kludex reviewed Aug 11, 2020

View reviewed changes

Remove repeated import

2202dc6

tiangolo added 2 commits July 29, 2021 17:22

🎨 Update import style to improve typing support

5e15af1

🔀 Merge master

3866e3d

tiangolo changed the title ~~Import types used by Request properties~~ ✨ Import and re-export data structures from Starlette, used by Request properties, on fastapi.datastructures Jul 29, 2021

tiangolo merged commit 4eada92 into tiangolo:master Jul 29, 2021

tiangolo mentioned this pull request Jul 29, 2021

Add imports of Starlette datastructures for type annotating Request #1871

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Import and re-export data structures from Starlette, used by Request properties, on `fastapi.datastructures` #1872

✨ Import and re-export data structures from Starlette, used by Request properties, on `fastapi.datastructures` #1872

jamescurtin commented Aug 10, 2020

codecov bot commented Aug 10, 2020 •

edited

github-actions bot commented Aug 10, 2020

SirTelemak commented Aug 10, 2020

jamescurtin commented Aug 11, 2020

Kludex left a comment

Kludex Aug 11, 2020

jamescurtin Aug 11, 2020

Kludex Aug 11, 2020

jamescurtin Aug 11, 2020

Kludex Aug 11, 2020

jamescurtin Aug 11, 2020

github-actions bot commented Aug 11, 2020

github-actions bot commented Aug 11, 2020

jamescurtin commented Aug 25, 2020

github-actions bot commented Jul 29, 2021

tiangolo commented Jul 29, 2021

		@@ -1,5 +1,11 @@
		from typing import Any, Callable, Iterable, Type

		from starlette.datastructures import URL # noqa: F401

-from starlette.datastructures import URL  # noqa: F401
+from starlette.datastructures import (
+    Address,
+    FormData,
+    Headers,
+    QueryParams,
+    State,
+    UploadFile as StarletteUploadFile,
+    URL
+)

		@@ -1,7 +1,6 @@
		"""FastAPI framework, high performance, easy to learn, fast to code, ready for production"""

		__version__ = "0.61.0"

		@@ -0,0 +1 @@
		from starlette.types import Receive # noqa: F401

✨ Import and re-export data structures from Starlette, used by Request properties, on fastapi.datastructures #1872

✨ Import and re-export data structures from Starlette, used by Request properties, on fastapi.datastructures #1872

Conversation

jamescurtin commented Aug 10, 2020

codecov bot commented Aug 10, 2020 • edited

Codecov Report

github-actions bot commented Aug 10, 2020

SirTelemak commented Aug 10, 2020

jamescurtin commented Aug 11, 2020

Kludex left a comment

Choose a reason for hiding this comment

Kludex Aug 11, 2020

Choose a reason for hiding this comment

jamescurtin Aug 11, 2020

Choose a reason for hiding this comment

Kludex Aug 11, 2020

Choose a reason for hiding this comment

jamescurtin Aug 11, 2020

Choose a reason for hiding this comment

Kludex Aug 11, 2020

Choose a reason for hiding this comment

jamescurtin Aug 11, 2020

Choose a reason for hiding this comment

github-actions bot commented Aug 11, 2020

github-actions bot commented Aug 11, 2020

jamescurtin commented Aug 25, 2020

github-actions bot commented Jul 29, 2021

tiangolo commented Jul 29, 2021

✨ Import and re-export data structures from Starlette, used by Request properties, on `fastapi.datastructures` #1872

✨ Import and re-export data structures from Starlette, used by Request properties, on `fastapi.datastructures` #1872

codecov bot commented Aug 10, 2020 •

edited