REF: Modify custom CRS classes to be functions that return CRS

[snowman2]

• Closes Pickle fails to load Pyproj objects #897
• Tests added
• Fully documented, including history.rst for all changes and api/*.rst for new API

Was thinking making the CRS maker classes into builders for the CRS objects would make things simpler for #847

[codecov]

Codecov Report

Merging #898 (9ebd23f) into master (95f97f6) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 9ebd23f differs from pull request most recent head f884e9d. Consider uploading reports for the commit f884e9d to get more accurate results

@@            Coverage Diff             @@
##           master     #898      +/-   ##
==========================================
+ Coverage   95.22%   95.27%   +0.04%     
==========================================
  Files          21       21              
  Lines        1677     1671       -6     
==========================================
- Hits         1597     1592       -5     
+ Misses         80       79       -1     

Impacted Files	Coverage Δ
pyproj/crs/crs.py	97.66% <100.00%> (+0.19%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95f97f6...f884e9d. Read the comment docs.

[snowman2]

Another alternative would be to create a separate base class for all of the maker CRS classes that handles the pickle & from_* stuff. I may tinker with that option as I am not entirely sure how I feel about this solution.

[jorisvandenbossche]

One consequence of this change is also that things like isinstance(crs, ProjectedCRS) will no longer work

[snowman2]

Yes, that one line sums up why I hesitate to make this change. The benefits of this change is that there is a single CRS class to manage/maintain without any of the wierdness that has come up due to differering behavior in the initialization. The downside is that you lose some of the benefits of it being a class such as inheritacne/isinstance stuff.

The question to answer is are the downsides reason enough to not go this way? For example, do we actually want the usage pattern to be isinstance(crs, ProjectedCRS) or would it be better to do this instead isinstance(crs, CRS) and crs.is_projected?

The lack of information for how people are currently using it makes me hesitate the most. Being more conservative on the change may not be a bad idea.

[jorisvandenbossche]

Yeah, I can't help with that question about what people are currently using (isinstance vs attribute check), that's always difficult to know.

Now, for the actual problem of the classes not being pickleable, why is it easier to fix this if they are a single class? Because that seems to suggest that you can have a single code path to reconstruct a class for pickling, which you should also be able to do on a base class and that then gets inherited by all classes?
(didn't look at the code to be clear, so I can certainly be missing something)

[snowman2]

Because that seems to suggest that you can have a single code path to reconstruct a class for pickling, which you should also be able to do on a base class and that then gets inherited by all classes?

#898 (comment)

[snowman2]

why is it easier to fix this if they are a single class?

This change also has #847 in mind. Having different arguments for the constructor is the current pain point. But, this could also be solved with the base class.

[snowman2]

Found a much less invasive solution: #901

[jorisvandenbossche]

Nice!