Opened 6 years ago
Closed 5 years ago
#30461 closed New feature (fixed)
contrib.gis.geoip2 does not support pathlib.Path as GEOIP_PATH parameter, only str
Reported by: | Nikita Krokosh | Owned by: | Nikita Krokosh |
---|---|---|---|
Component: | GIS | Version: | dev |
Severity: | Normal | Keywords: | geoip2 |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
I want to pass Path instance directly as GEOIP_PATH param but currently it only allowes str instances.
As I understand, min Python version for Django 2.2 is 3.5+ so I can freely pass Path instance there.
... # Getting the GeoIP data path. path = path or GEOIP_SETTINGS['GEOIP_PATH'] if not path: raise GeoIP2Exception('GeoIP path must be provided via parameter or the GEOIP_PATH setting.') if not isinstance(path, str): raise TypeError('Invalid path type: %s' % type(path).__name__) path = Path(path) ...
I've fixed this issue myself.
https://github.com/lsnk/django/tree/ticket_30461_2_2
https://github.com/django/django/pull/11341
Change History (11)
comment:1 by , 6 years ago
Description: | modified (diff) |
---|
comment:2 by , 6 years ago
Owner: | changed from | to
---|
follow-up: 4 comment:3 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 6 years ago
Replying to Claude Paroz:
Unless this is a regression from 2.1, this won't be backported to 2.2, so I'd suggest targeting the patch to the master branch.
I'm new to Django contributing, so can you please tell me, about the branches.
I thought the master branch is for 2.3+ versions, and 2.2 version is inside 2.2.x branch, so if I make PR into master the changes will come only with 2.3+ right?
I thought if I make them into 2.2 they'll hit next 2.2 hotfix and be backmerged into master.
comment:5 by , 6 years ago
The master branch is currently for Django 3.0. This is a new feature so it doesn't qualify for a backport to the Django 2.2 (see supported-versions) and even if patch qualifies for a backport it should be targeted to the master branch.
comment:6 by , 6 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
comment:7 by , 6 years ago
Version: | 2.2 → master |
---|
comment:8 by , 5 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
This PR is an alternate patch where I introduced a new to_path
utility that we could use in other cases (MEDIA_ROOT
, LOCALE_PATHS
, ...).
comment:9 by , 5 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Unless this is a regression from 2.1, this won't be backported to 2.2, so I'd suggest targeting the patch to the master branch.