Refactor deploy #326
Refactor deploy #326
Conversation
There was a problem hiding this comment.
thank you so much! I've been meaning to simplify this process since the beginning 😅
| mypy-python: | ||
| name: Static-types check | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - id: setup-python | ||
| name: Setup Python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.9' | ||
| cache: 'pipenv' | ||
|
|
||
| - id: install-pipenv | ||
| name: Install pipenv | ||
| run: curl https://raw.githubusercontent.com/pypa/pipenv/master/get-pipenv.py | python | ||
| shell: bash | ||
|
|
||
| - id: install-python-dependencies | ||
| name: Install Python dependencies | ||
| run: pipenv sync --dev | ||
| shell: bash | ||
|
|
||
| - id: mypy | ||
| name: Run mypy | ||
| run: pipenv run mypy ./datawrapper --ignore-missing-imports --verbose | ||
| shell: bash |
There was a problem hiding this comment.
mypy is a vestige of the original "hypermodern python package" template I used to create this repo back in the day (link).
I'm not sure that it's needed. Any opinions?
Happy to leave this in for now and take this up in a different conversation.
There was a problem hiding this comment.
I'm a fan of keeping it. We can beef up its implementation further on down the road. I use it for all my stuff at this point in a scheme similar to the one proposed here. Holds up great.
| types-requests = "*" | ||
|
|
||
| [requires] | ||
| python_version = "3.10" |
There was a problem hiding this comment.
I'm not super familiar with pipenv would this have a similar effect as python_requires=="3.10" in setup.py? Currently, we support 3.8 - 3.10. Just want to make sure we're not dropping 3.8 and 3.9 for the moment.
There was a problem hiding this comment.
The version specified there is the one that the developer of the application uses when making contributions. The testing regime intends to ensure that the tests pass and everything works in other versions.
There was a problem hiding this comment.
Yikes, I forgot this was here. I'll handle in another update.
| import time | ||
|
|
||
| from setuptools import setup | ||
| from setuptools_scm.version import guess_next_version |
There was a problem hiding this comment.
I see setup.py usage for setuptools_scm is deprecated: https://github.com/pypa/setuptools_scm#setuppy-usage-deprecated
It seems like they're leaning towards pyproject.toml. I don't have a lot of experience with pipenv but it seems like it could handle the pyproject.toml file. If we don't need to move to a setup.py configuration and we can stick to pyproject.toml I think we should. If just because more and more tools are moving in that direction.
There was a problem hiding this comment.
Yes, they are leaning that way for sure. I've just never gotten this kind of "auto releasing" scheme to work with a toml configuration. If you know how to do it, let's substituted it. It's just beyond my current knowledge.
There was a problem hiding this comment.
We can do that in a future release. Created #328 for this.
| importlib_metadata = "*" | ||
| rich = "*" | ||
| requests = "*" | ||
| pandas = "*" | ||
| ipython = "*" |
There was a problem hiding this comment.
might need to pin these in a future version
There was a problem hiding this comment.
Yes, perhaps in the setup.py install_requires list rather than the Pipfile, since it can act as the "bleeding edge" that runs against the tests between releases.
There was a problem hiding this comment.
Seems like this is not as simple as I thought. I'll revisit in another update.
|
One thing you'll need to do yet is create a PyPI API key with permission to update this package. Then set that |
|
Got it! |
This is a pretty drastic refactor that replaces the poetry scheme with pipenv. The idea is to add support for other version of Python, additional testing and continuous deployment to PyPI, and with significantly less code. Deploys would be push button, only requiring that click through a "release" in the GitHub interface. You don't lose any of your black and mypy and flake testing.
If it's too much, just let me know. Trying to smooth things out, but I may have gone too far.
Tests are passing here but it would surely require a little more work.
https://github.com/palewire/Datawrapper/actions/runs/4996143031
This aims to fix #123