Would base classes be useful?

[djhoese]

Looking through some of this package I noticed there aren't a lot of base classes or inheritance; like in the containers and datums modules. I'm not sure they would be useful, but at initial skimming of the code it may have made it easier to understand (map out in my mind). This is just my initial impression. I'm very impressed with everything that PyCRS is able to do and am excited to figure out how to incorporate it into my own code: https://satpy.readthedocs.io/en/latest/

Let me know if there is any way I can help. If you want to chat there is a link to join the satpy (pytroll community) slack here under "Getting in touch".

[karimbahgat]

Interesting idea. The obvious thing most objects have in common and that may benefit from inheritance is the standard to_proj4/ogc_wkt/esri_wkt() methods. This might be useful for some of the simpler classes, but other objects (those that contain other objects) typically require custom implementations of these methods anyway. Will have to think about this one, and please come with suggestions here.

The other aspect I think might benefit from this and need some rethinking, is how we implement the general container classes vs their specific instances, such as Ellipsoid() vs eg ellipsoids.WGS84, Datum() vs eg datums.NAD83, and Projection() vs eg projections.Robinson(). The specific instances contain some values that the general classes need to use, so maybe the general classes should simply subclass the specific ones. Don't remember right now my thinking around this, so need to go through and reconsider if it can be improved/simplified.

[djhoese]

Yeah I can see how what you talk about in the first paragraph doesn't make much sense with base classes. As for the Projection and Datum classes, I think this is what I was thining about. isinstance(obj, Datum) and isinstance(obj, Projection) and the classes could just be class Robinson(Projection):.

[karimbahgat]

Yeah, seems like we were thinking about the same things then. Will have a look at how it could be done.

[karimbahgat]

Had a first go at the subclassing now. Reshuffled so the Projection, Datum, and Ellipsoid base classes are now in their respective modules (rather than in 'containers'). The specific ones simply subclass the general ones, their usefulness being that they prepopulate some of the attributes so the user doesn't have to. See what you think @djhoese. Need to add Readme section on building from scratch.

Regarding the general base classes, since we're already reworking the API and upgrading the version, I'm curious which direction you think would be most intuitive (any pros or cons):

1. How it is now: If users want to create their own instances not already defined in the subclasses, they can use the base classes directly and populate their attributes (eg ellips = Ellipsoid(*args)).
2. Instead: Make the base classes private (eg class _Datum) not meant for end-users, and instead make a Custom subclass meant for end-users.

[karimbahgat]

Unless any additional input here, sticking with option 1 for the first release.

[karimbahgat]

Considering this closed, the class and subclass system makes more sense overall now.