[PARO-685] [PARO-744] Decouple CivisML from civis-python: Use template ID aliases and move docs to Zendesk

civisanalytics / civis-python

Civis API Python Client

BSD 3-Clause "New" or "Revised" License

34 stars 26 forks source link

[PARO-685] [PARO-744] Decouple CivisML from civis-python: Use template ID aliases and move docs to Zendesk #341

Closed jacksonllee closed 4 years ago

jacksonllee commented 4 years ago

This PR decouples CivisML from civis-python with respect to the following:

PARO-685: Use template ID aliases, so that a new CivisML release doesn't require a new civis-python release anymore.
PARO-744: Document CivisML dependency versions on Zendesk rather than in civis-python's Sphinx docs, so that we can update Zendesk independently when CivisML is updated for the modeling dependencies (civis-python's Sphinx docs don't update unless a new civis-python release is made).

@stephen-hoover You are the appointed reviewer, since I think you're the person who has thought about the use of aliases for this scenario the most.

jacksonllee commented 4 years ago

There's a couple very minor things for test failures that I'm going to fix, but this PR is still ready for review as-is.

jacksonllee commented 4 years ago

I've fixed the test failures related to this PR. The overall status for TravisCI is still red, because of all the following facts together:

Two hours ago, PyYAML released v5.3 (which bans Python 3.4).
civis-python allows PyYAML v5.3.
civis-python still runs tests on Python 3.4.

Only the build for Python 3.4 failed, and I'm going to ignore this failure in this PR. The solution would be to either restrict civis-python to use PyYAML < 5.3 (which doesn't seem desirable, since we're deprecating Python 3.4 support and eventually removing testing later anyway, cf. #339), or remove Python 3.4 from the Travis CI build yaml config when the civis-python maintainers officially do so.

stephen-hoover commented 4 years ago

It's really exciting to see this finally happening! I will get to code review sometime tomorrow. On the test failure, it looks like @uttercm is addressing the pyyaml dependency as part of #342 . That will need to merge before this can (or at the very least, we'll need the dependency updates from there).

uttercm commented 4 years ago

@jacksonllee my PR was merged so if you merge master you should be good to go

jacksonllee commented 4 years ago

Thanks for the help, @uttercm! All builds are green now.