Skip to content

Define pkg as a PackageURL class attribute #184

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.

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

Merged
merged 4 commits into from
Jun 6, 2025

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Mar 6, 2025

This will allow library users to subclass PackageURL and redefine the scheme to create PURL-like derivatives. (e.g. for the dep: idea described here):

class DependencyURL(PackageURL):
    SCHEME = "dep"

Which can be used in the same way and happily accepts PEP440-like version ranges in the version field 🥳:

>>> dep = DependencyURL.from_string("dep:pypi/requests@>=2.0")
>>> dep
DependencyURL(type='pypi', namespace=None, name='requests', version='>=2.0', qualifiers={}, subpath=None)

@jaimergp jaimergp closed this Mar 30, 2025
@pombredanne
Copy link
Member

@jaimergp this is fine by me BTW... no objections to make it easier to reuse!

@pombredanne pombredanne reopened this Apr 30, 2025
@pombredanne
Copy link
Member

Ah, I see, this is now with #186

@jaimergp
Copy link
Contributor Author

jaimergp commented May 8, 2025

Ah, @pombredanne, if you are fine with this type of change, yes, please, let's reopen!

@johnmhoran johnmhoran reopened this May 14, 2025
@jaimergp
Copy link
Contributor Author

@pombredanne (or anyone with merge rights!) gentle ping for a hopefully uncontroversial review!

@@ -314,6 +314,8 @@ class PackageURL(
https://github.com/package-url/purl-spec
"""

SCHEME: str = "pkg"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if using lower-caps scheme for consistency with existing vars would make more sense. I don't see anything declared as upper-caps in that module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used uppercase because it's supposed to be constant for the class, maybe I can indicate that with ClassVar[str] instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to ClassVar[str]

@jaimergp
Copy link
Contributor Author

jaimergp commented Jun 5, 2025

Thanks @tdruez, let me know if I addressed your concerns!

@tdruez
Copy link
Collaborator

tdruez commented Jun 5, 2025

@jaimergp Thanks! LGTM.

@pombredanne Could you give it another look and take care of the merge?

Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All fine! Thanks!

@tdruez tdruez merged commit d96295c into package-url:main Jun 6, 2025
19 checks passed
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.

4 participants