Uh oh!
There was an error while loading. Please reload this page.
resolve typing errors in project after merge - #331
Conversation
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
overall this is great as it uncovers a bunch of defects in the type declarations.
I'm a bit worried about the blast radius for anybody relying on static type checking? (I know not everyone does that in Python)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Andrew Omondi (andrueastman)
commented
Sep 11, 2024
Thanks for taking a look Vincent Biret (@baywet). I do also have a similar concern but feel like having the typing correct would be place we want to be at. e.g. There are some scenarios uncovered with implementations with more params than the parent class interface. What I can probably do as well is also do something similar to TS and generate a test project with Kiota and see what that uncovers as well. https://github.com/microsoft/kiota-typescript/tree/main/packages/test This will help understand if changes here are a big break. I'll set this as draft in the meantime to set that up if that makes sense. |
68e7caf to
e52fd69Compare
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
yes, having a smoke client with static type checking would help I believe.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
c710154 to
53ae508Compare53ae508 to
2227122Compare
This comment was marked as outdated.
This comment was marked as outdated.
Andrew Omondi (andrueastman)
commented
Sep 23, 2024
Created #333 to resolve parsable generation typing issues. |
Andrew Omondi (andrueastman)
commented
Sep 23, 2024
Vincent Biret (@baywet) Once those are closed, and this is good to go, I believe we can merge this one and now simply sort out the workflows to merge the repositories. |
Vincent Biret (baywet)
commented
Sep 23, 2024
Thanks for the follow up here. |
…dation adds validation with generated project
The short answer is not really. Builds are already broken(for users who try to consume the typing info of our packages) as we were really not publishing the typing information. The error/warning that shows up for current packages looks something like this. Skipping analyzing "kiota_serialization_form.form_parse_node_factory": module is installed, but missing library stubs or py.typed marker [import-untyped]According to PEP 561, for the type information to be consumed downstream, a package author either
If you take a look at the sources, none of them specify a What this means then is that for the consumer, currently, they would either
From the sources, it looks like we have been also suppressing type validation across projects and this explains why the typing issues discovered here were not surfacing across projects. See examples below (but this is across all projects)
As we are moving to a mono repo structure, the packages do not see each other as 3rd party anymore as they are co-located and thus the suppression won't hide these type errors anymore. So, if we wish to keep the type checking we would need to fix them. The end result of this process then will be
Hope this makes things clearer. |
Vincent Biret (baywet)
commented
Sep 24, 2024
Thank you so much for diving into this! We're effectively cleaning up a lot of defects in the types (or absence of) as the library as published. I guess some people might start seeing new errors surfacing up as the type system will kick in, but with this level of details it should be easy to justify we're actually fixing a major feature of kiota python clients. I'm guessing the graph packages are also missing the proper metadata at this stage? |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Vincent Biret (baywet)
commented
Oct 3, 2024
Andrew Omondi (@andrueastman) as a side note, this PR will also need to be replicated here microsoft/kiota-serialization-json-python#369 |
Andrew Omondi (andrueastman)
commented
Oct 7, 2024
Thanks for highlighting this Vincent Biret (@baywet) I've added this as a task in the bigger PR at #329 so that we try close this and not mix too many concerns here. I'll create a separate PR into the bigger PR once this one is closed. |
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Thank you for making the changes!


Partial of #302
Resolves failing mypy issues after repositories merging.
In general
CI for the PR at https://github.com/microsoft/kiota-abstractions-python/actions/runs/10807820407