Addresses #28, #37 moved main files to src folder. Testing and sphinx…

[maltevogl]

What should work?

• installing via pip install -U .
• Testing via tox
• Creating docs locally via tox -e docs

In principle ready for readthedocs (see yaml)

[maltevogl]

There might be changes necessary in the CI, since the package is now in a subfolder. This is somehow necessary? for setup.cfg

[rlskoeser]

@maltevogl it looks like the CI is failing because the path we were using for caching python deps has changed, I think you need to change it from the setup.py to the new setup.cfg in both the check and unit test workflows:

    cache-dependency-path: **/setup.py

[rlskoeser]

Thanks for doing all of this! Great to see. I was able to install locally, run tests with tox, and generate docs with tox – very nice!

Thanks for updating the readme; do you think we need any more details? (e.g., any problems if we still run tests with pytest instead of tox? what do we lose?) I guess when you generate the docs it gives you the path to view them.

I don't typically use tox on my projects, can you briefly state the value? I didn't know you could use it to build docs or run other commands, that seems convenient. (I only knew of it for running matrix-style testing on different versions locally.)

Do we want support for a matrix build with tox, and if so can we synchronize the python versions we're testing on GitHub Actions? (I'm not sure if this is worth the effort/time, I'm fine with running one version locally).

I think this generally looks good and we can probably merge once the checks are passing.

[rlskoeser]

I thought () wasn't needed anymore?

[maltevogl]

I am just old school, I guess. We can remove them

[rlskoeser]

I am too, trying to train myself not to do this style class Undate(Object): any more

[rlskoeser]

Do we still need a minimal setup.py for any legacy tooling? I thought I saw that somewhere in the docs but now I'm not seeing it.

[maltevogl]

I do not know how to change the CI for a PR only. Can someone look at this?

[rlskoeser]

I do not know how to change the CI for a PR only. Can someone look at this?

This is the syntax for making a step conditional so it only applies to pull requests:

    if: ${{ github.event_name == 'pull_request' }}

Which step needs to be made conditional for PRs only? I think the caching step change applies to all events.

Would be glad to help if you can provide more specifics about what you think is needed.

[ColeDCrawford]

@rlskoeser re setup.py, I think some tooling may still rely on it but can't remember what. The whole file can be this:

import setuptools
if __name__ == "__main__":
    setuptools.setup()

[rlskoeser]

@rlskoeser re setup.py, I think some tooling may still rely on it but can't remember what. The whole file can be this:

import setuptools
if __name__ == "__main__":
    setuptools.setup()

Thanks — I saw something like that somewhere in the docs but couldn't find it when I went back to look. That's what I thought — it's super minimal since it relies on the other config files, but still helpful to have it for some cases.

[rlskoeser]

@ColeDCrawford I don't know how to reconcile tox with the python version test matrix, do you ?

[ColeDCrawford]

Looks like tox-gh-actions worked!

[rlskoeser]

now do we need to specify the python versions in more places? would be nice if we could avoid that duplication somehow (seems redundant to have it in the tox config and also the build matrix). We should put it on the readme at some point, too, but hoping we can use a badge to pull from pypi once we publish.

[ColeDCrawford]

Agreed, it would be nice to pull the build matrix from tox.ini tox.envlist or setup.cfg, but not sure how to set that up.

[ColeDCrawford]

Think this looks great, looking forward to seeing docs. Tests are now passing with the updated tox and python matrix.

[rlskoeser]

Think this looks great, looking forward to seeing docs. Tests are now passing with the updated tox and python matrix.

@ColeDCrawford I don't see the GitHub Actions listed on the PR and the checks tab says there were zero workflow runs — any ideas?

[ColeDCrawford]

Think this looks great, looking forward to seeing docs. Tests are now passing with the updated tox and python matrix.

@ColeDCrawford I don't see the GitHub Actions listed on the PR and the checks tab says there were zero workflow runs — any ideas?

That is weird. When the tests were failing they appeared at the bottom of the PR, but you're right that I don't see them now. There's also no green check on https://github.com/dh-tech/hackathon-2022/pulls. But if you go to https://github.com/dh-tech/hackathon-2022/actions, you can see all the Actions runs, and this PR passes for both unit tests and style check

[ColeDCrawford]

Maybe related to the fact that the PR is from maltevogl:main to hackathon-2022 and not a branch of hackathon-2022? Not sure though

[rlskoeser]

@ColeDCrawford yes, I see the actions passed — very weird. They were showing up before, not sure why they stopped. (Possible your path filter is excluding it on the push, but I would have thought the pull would still run....) Should we just merge it and then deal with it if there are any problems?