dh-tech / undate-python

A Python library for working with fuzzy, partial, or otherwise uncertain dates
Apache License 2.0
8 stars 1 forks source link

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

Closed maltevogl closed 2 years ago

maltevogl commented 2 years ago

What should work?

In principle ready for readthedocs (see yaml)

maltevogl commented 2 years ago

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

rlskoeser commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

rlskoeser commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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?