-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
Add some more default module mappings #20641
Conversation
Mostly django-related, with some additional modules such as `python-dateutil` and `O365`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!
@@ -112,19 +112,34 @@ def two_groups_hyphens_two_replacements_with_suffix( | |||
"beautifulsoup4": ("bs4",), | |||
"bitvector": ("BitVector",), | |||
"cattrs": ("cattr", "cattrs"), | |||
"django-auditlog": ("auditlog",), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a lot (but not all) of these django-...
ones might be covered by the django-...
catch all rule above:
pants/src/python/pants/backend/python/dependency_inference/default_module_mapping.py
Line 88 in 73d41b4
re.compile(r"""^django-((.+(-.+)?))"""): [first_group_hyphen_to_underscore], |
In particular, the ones that map from django-foo-bar
to foo_bar
should be automatic. Could you adjust to only include the ones like django-oauth-toolkit
that don't have the simple mapping?
(NB. potentially you may observe different behaviour locally: that catch all was only added in Pants 2.19.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware of the catch-all (still using pants v2.17) - thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@huonw Note I've added two non-standard django cases (notifications
and actstream
) after removing the packages covered by the catch-all
src/python/pants/backend/python/dependency_inference/default_module_mapping.py
Outdated
Show resolved
Hide resolved
@@ -147,6 +162,7 @@ def two_groups_hyphens_two_replacements_with_suffix( | |||
"launchdarkly-server-sdk": ("ldclient",), | |||
"mail-parser": ("mailparser",), | |||
"mysql-connector-python": ("mysql.connector",), | |||
"o365": ("O365",), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, unconventional case. That's a "fun" choice by this library! 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed - no idea why the inconsistency!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
src/python/pants/backend/python/dependency_inference/default_module_mapping.py
Outdated
Show resolved
Hide resolved
src/python/pants/backend/python/dependency_inference/default_module_mapping.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Huon Wilson <[email protected]>
Mostly django-related, with some additional modules such as
O365
.