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.

Sign up for GitHub

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

Jump to bottom

Refactoring typing imports / usage #958

Closed
sveneberth opened this issue Nov 29, 2023 · 4 comments · Fixed by #986
Closed

Refactoring typing imports / usage #958

sveneberth opened this issue Nov 29, 2023 · 4 comments · Fixed by #986
Assignees
@ArneGudermann
Labels
discussion These topics must be discussed before completion idea Priority: Low This issue can be considered with enough idle time. syntax

Comments

@sveneberth
Copy link
Member

sveneberth commented Nov 29, 2023 •
edited
Loading

We have a lot of typing imports like from typing import this, that, ...

Sometimes they are really long:

viur-core/src/viur/core/skeleton.py

Line 12 in 8809c50

from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Type, Union

I saw in other projects they import typing aliased with t (ìmport typing as t) and write argument: t.Any.
Maybe we should do this in the viur-core too. What do you think, should this be part of our coding conventions?
@sveneberth sveneberth added discussion These topics must be discussed before completion Priority: Low This issue can be considered with enough idle time. idea syntax labels Nov 29, 2023
@ArneGudermann
Copy link
Contributor

I think that's a good idea. Let's do it this way, it's much cleaner.

@sveneberth sveneberth added the viur-meeting Issues to discuss in the next ViUR meeting label Dec 5, 2023
@sveneberth
Copy link
Member Author

sveneberth commented Dec 6, 2023 •
edited
Loading

We should also decide for one task decorator syntax. Currently we have

this (where we import the entire tasks module):

viur-core/src/viur/core/securitykey.py

Line 114 in 09312ee

@tasks.CallDeferred

and this (where we import every member of the module):

viur-core/src/viur/core/skeleton.py

Line 1317 in 09312ee

@CallDeferred

@phorward
Copy link
Member

phorward commented Dec 8, 2023

We should use t for Typing and tasks.* for task-related decorators, but not for standard decorators.

@ArneGudermann ArneGudermann mentioned this issue Dec 11, 2023
Merged
@phorward phorward removed the viur-meeting Issues to discuss in the next ViUR meeting label Jan 3, 2024
phorward added a commit that referenced this issue Jan 9, 2024
@ArneGudermann @phorward @sveneberth
refactor: Imp typing with t (#986)
37be9aa 
Resolve the typing part of #958

---------

Co-authored-by: Jan Max Meyer <jmm@phorward.de>
Co-authored-by: Sven Eberth <mail@sveneberth.de>
phorward added a commit that referenced this issue Jan 9, 2024
@ArneGudermann @phorward @sveneberth
refactor: Improved type annotations (#986)
30e9ced 
Mostly replaced `typing` by `t`
Resolve the typing part of #958

---------

Co-authored-by: Jan Max Meyer <jmm@phorward.de>
Co-authored-by: Sven Eberth <mail@sveneberth.de>
phorward added a commit that referenced this issue Jan 9, 2024
@ArneGudermann @phorward @sveneberth
refactor: Improved type annotations (#986)
3aa8fe8 
Mostly replaced `typing` by `t`
Resolve the typing part of #958

---------

Co-authored-by: Jan Max Meyer <jmm@phorward.de>
Co-authored-by: Sven Eberth <mail@sveneberth.de>
@phorward
Copy link
Member

phorward commented Jan 9, 2024

fixed by #986

@phorward phorward closed this as completed Jan 9, 2024
@sveneberth sveneberth linked a pull request Jan 11, 2024 that will close this issue
refactor: Improved use of type annotations, replace typing by t #986
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ArneGudermann ArneGudermann

Labels
discussion These topics must be discussed before completion idea Priority: Low This issue can be considered with enough idle time. syntax
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: Improved use of type annotations, replace typing by t ArneGudermann/viur-core
5 participants
@skoegl @sveneberth @phorward @akelch @ArneGudermann