CRS classmethods changed to staticmethods

[nmaxwell]

At some point pyproj used classmethods for constructor-type methods in pyproj.crs.CRS. In this commit this was changed to use staticmethods. The linked issue does not seem to mention this change (unless I missed it). In any case, this change effectively breaks subclassing, as in this example:

import pyproj


class myCRS(pyproj.crs.CRS):
    def print_foo(self):
        print("foo-bar!")


proj = pyproj.Proj("+proj=etmerc +ellps=GRS80 +lon_0=13 +lat_0=2 +x_0=0 +y_0=0 +k_0=1")
crs = myCRS.from_proj4(proj.definition_string())
print(type(crs))
print(isinstance(crs, myCRS))
crs.print_foo()

Expected Output

expect:

<class '__main__.myCRS'>
True
foo-bar!

what you get:

<class 'pyproj.crs.crs.CRS'>
False
...
AttributeError: 'CRS' object has no attribute 'print_foo'

Environment Information

Any ( we had been using pyproj v2.4.1, and just upgraded to 3.0.1)

Installation method

build from source

[snowman2]

This is how datacube does it: ref.

[snowman2]

Side note: if you use __init__ it should work:

proj = pyproj.Proj("+proj=etmerc +ellps=GRS80 +lon_0=13 +lat_0=2 +x_0=0 +y_0=0 +k_0=1")
crs = myCRS(proj.crs)

[snowman2]

Other methods to consider:

• to_3d
• geodetic_crs
• source_crs
• target_crs
• sub_crs_list

Also, need to ensure these methods still work with:

• GeographicCRS
• DerivedGeographicCRS
• ProjectedCRS
• VerticalCRS
• CompoundCRS
• BoundCRS

[nmaxwell]

Thanks for reviewing. OK i can see that in some cases super().__init__(...) would work in __init__, but not all. For instance from_proj4 calls _prepare_from_string first, and from_dict calls _prepare_from_dict first. I'm searching our codebase, and we actually call almost all of these different factory methods.

There's also something semantically clear that's lost when always doing through the generic __init__; from_epsg makes things very explicit and Pythonic.

---

Other methods to consider:

• to_3d
• geodetic_crs
• ...

So I've handled that situation elsewhere in the following way:

    def to_3d(self, name: Optional[str] = None) -> "CRS":
        # not-subclass-friendly: 
        # return CRS(self._crs.to_3d(name=name))
        # subclass friendly:
        return self.__class__(self._crs.to_3d(name=name))

I'm happy to add changes from explicit CRS(... to self.__class__, if you'd like. Please advise.

---

Also, need to ensure these methods still work with:

• GeographicCRS
• DerivedGeographicCRS
  ...

Those classes seem to all call CRS.__init__, not the factor methods, so they should be unaffected by switching the factory metods back to classmethods. I'm assuming this is all covered by tests.

That said, calling VerticalCRS.from_proj4 now does strange things:

old behavior:

crs = pyproj.crs.VerticalCRS.from_epsg(4326)
type(crs)
> pyproj.crs.crs.CRS

new behavior:

pyproj.crs.VerticalCRS.from_epsg(4326)
TypeError: __init__() missing 1 required positional argument: 'datum'

This makes sense: it basically calls pyproj.crs.VerticalCRS(4326), would gives the same error. I would argue that the old behavior is not really correct either: VerticalCRS.from_epsg should return a VerticalCRS.

What would be the correct behavior here? Should this be a separate issue?

Rough guess at the answer: VerticalCRS.from_epsg(4326) should raise an error since EPSG:4326 is not a vertical CRS, but VerticalCRS.from_epsg(5703) should be OK (and you should get an instance of VerticalCRS), since EPSG:5703 is a vertical CRS.

[snowman2]

What would be the correct behavior here?

I think that it would be better to remove from_* methods from inherited classes (i.e. VerticalCRS)

[snowman2]

I'm happy to add changes from explicit CRS(... to self.__class__, if you'd like.

That would make sense.

[snowman2]

Should this be a separate issue?

Probably fine to keep it connected here as changes made here will impact them.

[snowman2]

I'm happy to add changes from explicit CRS(... to self.__class__, if you'd like.

This will break CompoundCRS ... Will likely need a separate base class for these classes for this to work properly.

[snowman2]

Have a potential solution in #902