Skip to content

Conversation

@yangskyboxlabs
Copy link

Add annotations for P4 and P4API modules from p4python (https://pypi.org/project/p4python/) Annotations for P4.P4 class is very incomplete.

Having now done the work to extract typing information from our internal p4python wrapper, I've become less convinced in the value of attempting to provide comprehensive annotation for this library. Accurately annotating P4.P4 actually removes a lot of the utility of having these types in the first place: the implementation is simply too flexible in how it operates, owning to maintaining so many years of backwards compatibility and less-than-ideal architectural decisions. The wrapper I extracted this from is much more opinionated, limited in scope, and removes a lot of flexibility to for the sake of maintainability.

I see three ways forward with this PR, which I'd like to get some feedback on:

1. As-is

Accept roughly in its current shape, with the understanding that P4.P4 will likely never be more accurately annotated. It will potentially not receive much community attention in terms of continued investment, despite it having tens of thousands of downloads a day.

Speaking personally, I would much rather put effort towards polishing and open sourcing an opinionated wrapper that I would be happy to put my and my employer's name on.

2. Abandon attempting to annotate P4.run_*(), P4.fetch_*(), P4.save_*() et al

The main pain point is that P4.P4 uses __getattr__() to materialize all supported commands as methods. The effective surface area is huge, but not all known commands can operate in every mode. Each command also require or accept different parameters.

However, there is very little churn in the API from release-to-release. The foundational classes implemented in C++ (P4API.P4Adapter, P4API.P4Map et al) also see very little change, if at all. There are still useful annotations outside of P4.P4 that will be critical for anyone attempting to wrap p4python with a better API for their own use. These classes are annotated comprehensively, and, I feel, have value.

For now, I think this is my preferred way forward.

3. Abandon for now

Given that it's proven not feasible for us to significantly down-scope our internal wrapper library in favour of good type annotation on p4python itself, I'm content to chalk this up to a useful learning experience and move on.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

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.

1 participant