Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build failure #34

Merged
merged 13 commits into from
Oct 9, 2024
Merged

Fix build failure #34

merged 13 commits into from
Oct 9, 2024

Conversation

jeffjennings
Copy link
Contributor

@jeffjennings jeffjennings commented Oct 2, 2024

The tests on main are currently failing, and the setup.cfg is causing problems for the learn-astropy CI that depends on this package. This PR gets the tests passing and updates setup.cfg. Changes:

  • Updates requirements in setup.cfg:

    • Update version of dependency async_timeout - based on this algolia .toml
    • In dev requirements, replace types-pkg_resources (deprecated, see here) with types-setuptools
  • Uses pydantic.v1 throughout code, where previously a couple outlier instances of pydantic were used (which was causing multiple problems in tests)

  • Updates python versions for tests

  • Skips typing and linter CI steps, as there are several (probably benign) typing complaints and the linter step was just force-running pre-commit with a github action that is no longer being updated (.pre-commit-config.yaml is still present in the repo). These steps can be uncommented in the workflow .yaml in the future if desired.

@jeffjennings jeffjennings requested review from kelle and adrn October 9, 2024 14:21
Copy link
Member

@kelle kelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks AMAZING to me! Thanks so much for working on this!

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment. Otherwise LGTM.

p.s. In the future, please open PRs from your fork. Thanks!

@jeffjennings jeffjennings removed the request for review from adrn October 9, 2024 19:51
@jeffjennings jeffjennings merged commit 123fb48 into main Oct 9, 2024
6 checks passed
@jeffjennings jeffjennings deleted the jeffjennings-patch-1 branch October 9, 2024 19:52
@jeffjennings jeffjennings mentioned this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants