fix: design and type hint of custom collections

[moe-ad]

Closes #2288.

The following changes have been made:

• Making the fact that Collection is also a generic type more explicit. From my investigation, this helps older type checker versions (especially on older python versions) infer types more accurately.
• Make the CollectionFactory function a collection_factory class method of Collection. This is done for three main reasons:
  • It is makes more sense and it is more intuitive to bind it to the class, making it more obvious that it returns class objects deriving from Collection.
  • Eliminating the need to pass an explicit BaseClass argument. I couldn't think of a reason why a different argument other than Collection will be passed. @PProfizi what do you think?
  • To accurately type-hint the factory function, access to the type parameter of Collection is needed.
• Corrections and improvements to the type hints.

With these changes, IDEs are now better able to suggest accurate autocompletion. Example in PyCharm:

The type inference is also much more accurate. For example, IDEs are able to infer that a StringFieldsCollection contains StringFields:

And because of that, the type of the object returned by a call to get_entry will be inferred correctly as StringField:

And only available methods of that object will be suggested through autocompletion:

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.36%. Comparing base (3323383) to head (0ae8d19).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2695   +/-   ##
=======================================
  Coverage   84.36%   84.36%           
=======================================
  Files          92       92           
  Lines       10890    10890           
=======================================
  Hits         9187     9187           
  Misses       1703     1703           

[jorgepiloto]

Just left some suggestions. Thanks for adding this, @moe-ad.

[jorgepiloto]

Let's use T as per the original PEP 484:

Suggested change

	TYPE = TypeVar("TYPE")
	T = TypeVar("T")

[moe-ad]

I would prefer leaving this as is. Just to conform with the naming convention adopted for type parameters in other modules.

[PProfizi]

@moe-ad could we maybe add a comment about the presence of Generic[TYPE], and a docstring example for the collection_factory static method?

[moe-ad]

@moe-ad could we maybe add a comment about the presence of Generic[TYPE], and a docstring example for the collection_factory static method?

@PProfizi I added dc4c274. Is that ok?

[PProfizi]

@moe-ad could we maybe add a comment about the presence of Generic[TYPE], and a docstring example for the collection_factory static method?

@PProfizi I added dc4c274. Is that ok?

Yes that's perfect, thanks @moe-ad !

[github-actions]

Some tests with 'continue-on-error: true' have failed:

• PyDPF-Post API tests on ubuntu-latest

Created by continue-on-error-comment