fix: qualifiers type annotation

[jhoward-lm]

This PR modifies the type annotation of the qualifiers attribute declaration from:

qualifiers: Union[str, Dict[str, str], None]

to:

qualifiers: Dict[str, str]

in order to reflect its true runtime type.

Closes #169.

Miscellaneous

• fixed return type of the encode: "Optional[Literal[False]]" overload of normalize_qualifiers to Dict[str, str]
• fixed return type of the encode: "Optional[Literal[False]]" overload of normalize to Dict[str, str] (the other tuple members in the return type unchanged)
• updated instances of Union[AnyStr, Dict[str, str], None] to Optional[Union[AnyStr, Dict[str, str]]]
• updated instances of Union[str, Dict[str, str], None] to Optional[Union[str, Dict[str, str]]]
• removed unused # NOQA directives
• updated return type of PackageURL.__new__ and PackageURL.from_string to typing_extensions.Self
• updated first param name of PackageURL.__new__ from self to cls since it is a classmethod
• removed quotes from type annotation forward references by adding from __future__ import annotations

[jhoward-lm]

@tdruez @gruebel I don't have permissions to add reviewers, please take a look when you get a chance. Thanks!

[gruebel]

looks good and locally it passes mypy 😄

[tdruez]

@jhoward-lm I see there are still Union[str, Dict[str, str], None] references in the source, should those be updated to Dict[str, str] as well?

[jhoward-lm]

@tdruez I don't believe so. Those are generalized argument/parameter types, but the type gets narrowed before returning.

You did however draw my attention to several other typing issues; I'll push a commit shortly for review

[jhoward-lm]

@tdruez @gruebel With my most recent comment, tests still pass locally, although the oldest version of python I was able to test with was 3.9 (3.7 and 3.8 are EOL so I couldn't install them with homebrew).

I'll update the PR description with more detail. Let me know your thoughts!

[jhoward-lm]

@gruebel @tdruez
With the addition of from __future__ import annotations, the pipe style union operator as well as built-in collection types (Dict -> dict, Tuple -> tuple, etc.) are available. I verified by running pyupgrade --py37-plus src/packageurl/__init__.py.

So things like this could be updated across the board:

# before
Union[str, Dict[str, str], None]

# after
str | dict[str, str] | None

Let me know if you want me to go ahead with it, or I can hold off.

[gruebel]

I actually like the newer syntax style more than the old one, but I would recommend to do it in a follow-up PR, because it is kind of like formatting and I don't like to mix actual changes with formatting.

[jhoward-lm]

@tdruez @gruebel I was able to get python 3.7 installed using pyenv to attempt to test, but in case you aren't aware, running make virtualenv fails with

ImportError: cannot import name 'cached_property' from 'functools'

[gruebel]

@jhoward-lm there was no need for testing it with 3.7, because it was dropped with the last release. And CI passed already with your changes, so no need to do extra testing for this PR 😄

[jhoward-lm]

@tdruez @gruebel Is there anything else I need to do for this PR? Thanks for reviewing!

[jhoward-lm]

@tdruez @gruebel I threw together a quick branch on my fork to test caching for faster lookup. Basically repeated calls to __new__ with the same arguments return the previously constructed PackageURL object that was instantiated using those same arguments.

If you're interested, let me know if you want me to create an issue or a draft PR for review.

Tested with this against the current code and after implementing the caching:

if __name__ == "__main__":
    import timeit

    def _test_parsing():
        purl = PackageURL.from_string(
            "pkg:deb/ubuntu/ca-certificates@20230311?arch=all&distro=lunar"
        )

    print("time:", timeit.timeit(stmt=_test_parsing))

Current code: ~12.8 seconds
With caching: ~8.5 seconds

The results were a bit more dramatic when I tested with this:

if __name__ == "__main__":
    import timeit

    def _test_parsing():
        purl = PackageURL(
            type="deb",
            namespace="ubuntu",
            name="ca-certificates",
            version="20230311",
            qualifiers="arch=all&distro=lunar",
            subpath=None,
        )

    print("time:", timeit.timeit(stmt=_test_parsing))

Current code: ~5.9 seconds
With caching: ~0.5 seconds

Previous test but with qualifiers={"arch": "all", "distro": "lunar"}:

Current code: ~5.1 seconds
With caching: ~0.9 seconds

[jhoward-lm]

@tdruez @gruebel Disregard the last comment, since it's not relevant to this PR. Are you good with the PR as is?

[tdruez]

Are you good with the PR as is?

@jhoward-lm All good, PR merged. Thanks for your contribution 👍