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.

Sign up for GitHub

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

Jump to bottom

Small docs updates #1642

Merged
merged 9 commits into from
May 24, 2022
Merged

Small docs updates #1642

merged 9 commits into from
May 24, 2022

Conversation

aesharpe
Copy link
Member

@aesharpe aesharpe commented May 19, 2022 •
edited

  • Fix links
  • Integrate Settings File info into Running the ETL docs page
  • Remove old Settings File from docs (and indexes)
  • Add some comment headers to the actual deployed settings files to clarify which settings pertain to which script

@aesharpe
Fix small docs errors:
f5f4351 
- Fix links
- Remove tripple t from Setttings
- Add some clarity to the Settings Files page
- Update Install conda section to install mamba
@aesharpe aesharpe added the docs Documentation for users and contributors. label May 19, 2022
@aesharpe aesharpe requested a review from zaneselvans May 19, 2022 17:48
@aesharpe aesharpe self-assigned this May 19, 2022
@aesharpe aesharpe changed the title Fix small docs errors: Small docs updates May 19, 2022
@codecov
Copy link

codecov bot commented May 19, 2022 •
edited

Codecov Report

Merging #1642 (2d74ff5) into dev (2bd7831) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##             dev   #1642   +/-   ##
=====================================
  Coverage   84.0%   84.0%           
=====================================
  Files         65      65           
  Lines       7176    7176           
=====================================
  Hits        6034    6034           
  Misses      1142    1142
Impacted Files Coverage Δ
src/pudl/cli.py 67.5% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bd7831...2d74ff5. Read the comment docs.

@aesharpe
Update settings docs:
24d743b 
- Combine Settings Files and Running the ETL Pipeline docs pages
- Add some new and clarifying content to Running the ETL Pipeline docs
- Remove the old Settings Files docs page (also from the index)
@aesharpe
Update settings files so that there is a clear header/distinction bet…
a41840d 
…ween the sections for each script
@aesharpe aesharpe marked this pull request as ready for review May 19, 2022 23:39
zaneselvans
zaneselvans requested changes May 20, 2022
View reviewed changes
Copy link
Member

@zaneselvans zaneselvans left a comment

Choose a reason for hiding this comment

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

Found one mamba command that has to be conda, and a small copyedit.

docs/dev/dev_setup.rst Outdated
$ conda update conda
$ conda config --set channel_priority strict
$ mamba update mamba
$ mamba config --set channel_priority strict
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this command doesn't actually work. Has to be conda config ...

Copy link
Member Author

Choose a reason for hiding this comment

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

ooo good catch!

docs/dev/run_the_etl.rst Outdated
settings:

- EPA CEMS cannot be loaded without EIA data unless you have existing PUDL database
containing EIA. This is because CEMS because it relies on IDs from EIA860
Copy link
Member

Choose a reason for hiding this comment

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

copy edit "because CEMS because..."

@zaneselvans
Copy link
Member

Note that the docs build is failing because there are still references to the now removed settings page.

@aesharpe
Copy link
Member Author

aesharpe commented May 21, 2022 •
edited

Note that the docs build is failing because there are still references to the now removed settings page.

It seems like the error is due to some :download: commands in the data_source pages:
Running tox -e docs

docs run-test: commands[2] | rstcheck --config tox.ini --recursive ./
./docs/data_sources/eia923.rst:46: (INFO/1) No role entry for "download" in module "docutils.parsers.rst.languages.en".
./docs/data_sources/ferc1.rst:40: (INFO/1) No role entry for "download" in module "docutils.parsers.rst.languages.en".
./docs/data_sources/wip_future.rst:46: (INFO/1) No role entry for "download" in module "docutils.parsers.rst.languages.en".
./docs/data_sources/eia860.rst:42: (INFO/1) No role entry for "download" in module "docutils.parsers.rst.languages.en".
ERROR: InvocationError for command /Users/aesharpe/Desktop/Work/Catalyst_Coop/Repos/pudl/.env_tox/bin/rstcheck --config tox.ini --recursive ./ (exited with code 1)
______________________________________________ summary _______________________________________________
ERROR:   docs: commands failed

The links look fine, I'm not sure what a "role entry" is. It's coming from this section of the code, for example, in docs/data_sources/eia923.rst:

* :download:`EIA-923 Instructions (PDF, to 2013-10-31)
  <eia923/eia923_instructions_2013-10-31.pdf>`
* :download:`EIA-923 Instructions (PDF, to 2015-12-31)
  <eia923/eia923_instructions_2015-12-31.pdf>`
* :download:`EIA-923 Instructions (PDF, to 2017-05-31)
  <eia923/eia923_instructions_2017-05-31.pdf>`
* :download:`EIA-923 Instructions (PDF, to 2020-03-31)
  <eia923/eia923_instructions_2020-03-31.pdf>`
* :download:`EIA-923 Instructions (PDF, to 2023-05-31)
  <eia923/eia923_instructions_2023-05-31.pdf>`

Seems like we may need to update some package or something?

zaneselvans
zaneselvans reviewed May 23, 2022
View reviewed changes
docs/dev/run_the_etl.rst

.. seealso::

* :ref:`install-workspace` for more on how to create a PUDL data workspace.
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to remove this link to the install-workspace page?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I added the link up higher at the beginning of the Settings section in line 32

@zaneselvans zaneselvans self-requested a review May 24, 2022 17:26
@aesharpe aesharpe merged commit 0b70f76 into dev May 24, 2022
@cmgosnell cmgosnell deleted the docs-updates branch August 31, 2022 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zaneselvans zaneselvans zaneselvans approved these changes

Assignees

@aesharpe aesharpe

Labels
docs Documentation for users and contributors.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@aesharpe @zaneselvans