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

(re) draft Census connector #978

Merged
merged 14 commits into from
Feb 20, 2024
Merged

(re) draft Census connector #978

merged 14 commits into from
Feb 20, 2024

Conversation

Charlie-Kramer
Copy link
Contributor

Draft census connector--fixed typo in index.html (thanks Austin!), linted, re-ran tests.

@shaunagm shaunagm added the 🎉 first PR the first PR by a new contributor label Jan 25, 2024
@shaunagm
Copy link
Collaborator

shaunagm commented Feb 8, 2024

Hi Charlie - looks like you're still having issues with linting. Let me know if I can help!

@Charlie-Kramer
Copy link
Contributor Author

Shauna thanks for the heads up! I managed to fix it but now I'm getting this error (under build)

Sphinx version error:
The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.

I see some discussion of this error on stackoverflow (https://stackoverflow.com/questions/77848565/sphinxcontrib-applehelp-breaking-sphinx-builds-with-sphinx-version-less-than-5-0) but I'm not sure what to do here, grateful for any pointers!

Charlie

@shaunagm
Copy link
Collaborator

shaunagm commented Feb 8, 2024

Looks like this is an issue that Soren submitted a PR a couple weeks ago to fix: #975

If you update your branch, that should fix the issue, although note once you do that you'll want to pull those changes down to your local branch before making any additional edits.

Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

This looks great! I tested it out following your documentation and it's working for me :) Just a couple minor edits to make and we should be good to merge

docs/census.rst Outdated
Overview
********

Connects to the Census API--it has been tested with the ACS and Economic Survey endpoints.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a link to the Census API here? Also, this is super nitpicky, but do you mind editing this so there's a period instead of a double dash between the two sentences? Sorry to be such a grammar nerd 🤓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Shauna! I fixed these and the other stuff. For the index.html file, I just downloaded the original and replaced my banged-up version :).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks great! Going to merge now.

docs/census.rst Outdated Show resolved Hide resolved
docs/census.rst Outdated Show resolved Hide resolved
docs/census.rst Outdated Show resolved Hide resolved
docs/index.html Outdated Show resolved Hide resolved
parsons/census/census.py Outdated Show resolved Hide resolved
@shaunagm shaunagm merged commit cc84320 into move-coop:main Feb 20, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 first PR the first PR by a new contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants