Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#13315 closed (fixed)

Backwards-incompatible changes for GeoDjango not documented (e.g., PostGISAdaptor -> PostGISAdapter)

Reported by: bkonkle Owned by: jbronn
Component: GIS Version: 1.2-beta
Severity: Keywords: geodjango, gis, postgis, PostGISAdaptor
Cc: brandon.konkle@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The django.contrib.gis.db.backend.postgis.adaptor.PostGISAdaptor class has been renamed to django.contrib.gis.db.backend.postgis.adapter.PostGISAdapter in 1.2, but this is not documented in http://docs.djangoproject.com/en/dev/releases/1.2/. I'm attaching a patch that will add a quick note about it.

Attachments (4)

postgisapapterchange.diff (1.0 KB) - added by bkonkle 5 years ago.
postgisapapterchange-2.diff (1.0 KB) - added by bkonkle 5 years ago.
13315.1.diff (2.8 KB) - added by jbronn 5 years ago.
Provides backwards-compatibility aliases.
13315.2.diff (3.4 KB) - added by jbronn 5 years ago.
Added aliases to django.contrib.gis.models.

Download all attachments as: .zip

Change History (14)

Changed 5 years ago by bkonkle

comment:1 Changed 5 years ago by bkonkle

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Correction - I forgot the 's' on the end of 'backends' for the renamed class. I'm uploading a new diff.

Changed 5 years ago by bkonkle

comment:2 follow-up: Changed 5 years ago by russellm

  • Component changed from Documentation to GIS
  • Triage Stage changed from Unreviewed to Design decision needed

If this is a backwards incompatible change, it's not just a documentation issue - we need to look at whether the change is necessary at all. Pushing to the GIS so it get specific eyes from Justin.

comment:3 in reply to: ↑ 2 Changed 5 years ago by jbronn

  • Owner changed from nobody to jbronn
  • Status changed from new to assigned
  • Summary changed from Backwards-incompatible change not documented: PostGISAdaptor -> PostGISAdapter to Backwards-incompatible changes for GeoDjango not documented (e.g., PostGISAdaptor -> PostGISAdapter)

Replying to russellm:

If this is a backwards incompatible change, it's not just a documentation issue - we need to look at whether the change is necessary at all. Pushing to the GIS so it get specific eyes from Justin.

It's a matter of debate to whether this is a backwards-incompatible change -- the root cause of the issue is that SpatialBackend (and SpatialBackend.Adaptor) no longer exists due to multiple-database support, however, it was never documented to begin with.

I did indeed rename the Adaptor objects to the more common spelling of Adapter; providing aliases is a possibility -- however, I also changed the root namespace to django.contrib.gis.db.backends (the s was added), thus even the aliases wouldn't even be truly backwards-compatible.

Now that we're on the subject, I'm reminded of another change that must be documented: django.contrib.gis.models.SpatialRefSys no longer exists (again, due to multi-db support). This, and the GeometryColumns model were moved to each respective backend (e.g., django.contrib.gis.db.backends.postgis.models), and can also be accessed by methods on the connection (connection.ops.spatial_ref_sys, connection.ops.geometry_columns).

I agree that documenting these changes is necessary, however, all of the name changes have been done to internal APIs within GeoDjango. But I disagree with backing out of these changes now (minus possible model alias to default connection in django.contrib.gis.models) in the name of preserving compatibility for those relying on undocumented APIs.

comment:4 Changed 5 years ago by bkonkle

  • Cc brandon.konkle@… added

Changed 5 years ago by jbronn

Provides backwards-compatibility aliases.

comment:5 Changed 5 years ago by jbronn

  • Needs documentation set

Changed 5 years ago by jbronn

Added aliases to django.contrib.gis.models.

comment:6 Changed 5 years ago by Alex

Justin, can you add a note to the backwards compatibility timeline doc, so we can (amongst other things) remember when to remove these aliases.

comment:7 Changed 5 years ago by russellm

@jbronn: What's the status on this ticket? Is there a reason you uploaded a patch to the ticket rather than committing?

comment:8 Changed 5 years ago by jbronn

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [13097]) Fixed #13315, #13430 -- Recreated django.contrib.gis.db.backend module with SpatialBackend alias and added Adaptor alias for backwards-compatibility purposes; added GeoDjango 1.2 backwards-incompatibility documentation and release notes; added a section in the docs about how MySQL is a crippled spatial database; updated versions in install docs.

comment:9 Changed 5 years ago by anonymous

Justin: do you mind adding a note to internals/deprecation.txt?

comment:10 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top