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

Reference calitp-data-infra package as additional context in Python libraries doc #2856

Merged
merged 5 commits into from
Aug 4, 2023

Conversation

SorenSpicknall
Copy link
Contributor

Description

This PR adds a small subsection under the existing "Add New Packages" heading in the analyst-facing Useful Python Libraries guide, outlining the existence of the calitp-data-infra package and referencing the fact that it's used for some advanced tasks like authenticating sync pipelines. This is not meant to be comprehensive in any way, but provides an entry point to conceptual familiarity with calitp-data-infra and with our secrets handling in an easily approachable place.

Additionally, this PR makes some linting fixes to the existing markdown document that was edited.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

How has this been tested?

Markdown linted sucessfully

Post-merge follow-ups

  • No action required
  • Actions required (specified below)

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Comment on lines 31 to 32
5. [Add New Packages](#add-new-packages)
<br> - [calitp-data-infra](#calitp-data-infra)
Copy link
Contributor

@atvaccaro atvaccaro Aug 2, 2023

Choose a reason for hiding this comment

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

Should calitp-data-infra live in an appendix? the table of contents looks weird

Copy link
Contributor Author

@SorenSpicknall SorenSpicknall Aug 2, 2023

Choose a reason for hiding this comment

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

I grouped it under Add New Packages because the docs refer to the other packages as built into the JupyterHub environment, and I thought it would be useful to have the final section list other useful (but not automatically available) packages as subheadings. However, maybe I'm overemphasizing the bit of the docs about how the other packages come preinstalled on JupyterHub, and should just treat cal-itp-data-infra as another top-level heading? Happy to do what you feel makes the most sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I would suggest moving calitp-data-infra into an appendix. It really should be separate and unnecessary to install in the image for the most part.

docs/analytics_tools/python_libraries.md Outdated Show resolved Hide resolved
docs/analytics_tools/python_libraries.md Outdated Show resolved Hide resolved
docs/analytics_tools/python_libraries.md Outdated Show resolved Hide resolved
@SorenSpicknall SorenSpicknall merged commit 8ed0d4f into main Aug 4, 2023
2 checks passed
@SorenSpicknall SorenSpicknall deleted the reference-calitp-data-infra-in-packages-docs branch August 4, 2023 15:12
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.

2 participants