kylebarron / stata_kernel

A Jupyter kernel for Stata. Works with Windows, macOS, and Linux.
https://kylebarron.dev/stata_kernel/
GNU General Public License v3.0
264 stars 57 forks source link

Poetry setup #419

Closed kylebarron closed 2 years ago

kylebarron commented 2 years ago

Ref #416

A more modern setup with lockfile than using requirements.txt. See https://python-poetry.org/

Dependency locking of pywin32 is taking forever though (~30min so far)

kylebarron commented 2 years ago

This branch needs to be updated with master after #418 .

mcaceresb commented 2 years ago

@kylebarron This looks good to me; how to modify the workflows to work with poetry?

kylebarron commented 2 years ago

I'd suggest using this: https://github.com/snok/install-poetry

So the test ci would become

      - uses: actions/checkout@v2

      - name: Set up Python ${{ matrix.python-version }}
        uses: actions/setup-python@v2
        with:
          python-version: ${{ matrix.python-version }}

      - name: Set up Poetry
        uses: snok/install-poetry@v1

      - name: Install dependencies
        run: |
          poetry install
          poetry run python -m stata_kernel.install

      - name: Run tests
        # TODO: We should be running other tests and not just these two files
        run: |
          poetry run pytest tests/test_mata_lexer.py tests/test_stata_lexer.py
          # poetry run python setup.py install

Not sure off the top of my head how to edit the setup.py install step now that we don't have a setup.py file. What does setup.py install do? I thought it was equivalent to pip install .

mcaceresb commented 2 years ago

Not sure off the top of my head how to edit the setup.py install step now that we don't have a setup.py file. What does setup.py install do? I thought it was equivalent to pip install .

@kylebarron I think so. My read is that it's not needed for the tests, but I could be wrong.

kylebarron commented 2 years ago

My read is that it's not needed for the tests, but I could be wrong.

I don't think it's needed for the tests. I think it was just to test that the package could be installed

mcaceresb commented 2 years ago

@kylebarron

My read is that it's not needed for the tests, but I could be wrong.

I don't think it's needed for the tests. I think it was just to test that the package could be installed

Is there a command to install a local package with poetry? If yes then that could go in place; if no then the line can just be deleted, right?

kylebarron commented 2 years ago

Is there a command to install a local package with poetry?

This is just poetry install from the package root.