Fixing up readthedocs -- still in progress. #35
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #35 +/- ##
=======================================
Coverage 91.72% 91.72%
=======================================
Files 13 13
Lines 822 822
=======================================
Hits 754 754
Misses 68 68 ☔ View full report in Codecov by Sentry. |
WalkthroughThese changes introduce several improvements, including enhanced import structure using relative paths in various scripts within the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
…ormat is still very off though.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- .gitignore (1 hunks)
- README.md (3 hunks)
- docs/source/conf.py (1 hunks)
- src/MEDS_tabular_automl/scripts/cache_task.py (1 hunks)
- src/MEDS_tabular_automl/scripts/describe_codes.py (1 hunks)
- src/MEDS_tabular_automl/scripts/launch_xgboost.py (1 hunks)
- src/MEDS_tabular_automl/scripts/tabularize_static.py (1 hunks)
- src/MEDS_tabular_automl/scripts/tabularize_time_series.py (1 hunks)
- src/MEDS_tabular_automl/utils.py (1 hunks)
Files skipped from review due to trivial changes (2)
- .gitignore
- src/MEDS_tabular_automl/scripts/describe_codes.py
Additional context used
Ruff
docs/source/conf.py
22-22: Module level import not at top of file (E402)
src/MEDS_tabular_automl/scripts/tabularize_time_series.py
8-8: Module level import not at top of file (E402)
9-9: Module level import not at top of file (E402)
10-10: Module level import not at top of file (E402)
11-11: Module level import not at top of file (E402)
13-13: Module level import not at top of file (E402)
14-14: Module level import not at top of file (E402)
15-15: Module level import not at top of file (E402)
16-16: Module level import not at top of file (E402)
18-18: Module level import not at top of file (E402)
19-19: Module level import not at top of file (E402)
20-20: Module level import not at top of file (E402)
21-21: Module level import not at top of file (E402)
22-22: Module level import not at top of file (E402)
23-30: Module level import not at top of file (E402)
94-94: Function definition does not bind loop variable
agg(B023)
101-101: Function definition does not bind loop variable
window_size(B023)
102-102: Function definition does not bind loop variable
agg(B023)src/MEDS_tabular_automl/scripts/tabularize_static.py
13-13: Module level import not at top of file (E402)
15-15: Module level import not at top of file (E402)
17-22: Module level import not at top of file (E402)
23-23: Module level import not at top of file (E402)
24-24: Module level import not at top of file (E402)
25-25: Module level import not at top of file (E402)
26-34: Module level import not at top of file (E402)
142-142: Function definition does not bind loop variable
agg(B023)src/MEDS_tabular_automl/scripts/launch_xgboost.py
90-90: Remove unnecessary
True if ... else False(SIM210)Remove unnecessary
True if ... else False
LanguageTool
README.md
[uncategorized] ~59-~59: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...pts Overview 1.meds-tab-describe: This command processes MEDS data shards...
[uncategorized] ~72-~72: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...``` 2.meds-tab-tabularize-static: Filters and processes the dataset based...
[typographical] ~72-~72: The word “thus” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence. (THUS_SENTENCE)
Context: ...o a uniquepatient_idandtimestampcombination, thus rows are duplicated across multiple tim...
[uncategorized] ~84-~84: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...3.meds-tab-tabularize-time-series: Iterates through combinations of a shar...
[uncategorized] ~99-~99: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ax] ``` 4.meds-tab-cache-task: Aligns task-specific labels with the ne...
[grammar] ~101-~101: Possible subject-verb agreement error detected. (PLURAL_THAT_AGREEMENT)
Context: ...a specific task$TASKand labels that has pulled from [ACES](https://github.com/j...
[uncategorized] ~112-~112: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...e/max] ``` 5.meds-tab-xgboost: Trains an XGBoost model using user-spec...
[uncategorized] ~125-~125: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... ``` 6.meds-tab-xgboost-sweep: Conducts an Optuna hyperparameter sweep...
[uncategorized] ~129-~129: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... Scripts 1.generate-permutations: Generates and prints a sorted list of a...
[typographical] ~131-~131: After the expression ‘for example’ a comma is usually used. (COMMA_FOR_EXAMPLE)
Context: ... window sizes and aggregations. For example you can directly call **`generate-permu...
Markdownlint
README.md
19-19: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
36-36: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
150-150: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
167-167: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
169-169: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
23-23: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
29-29: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
31-31: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
68-68: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
76-76: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
88-88: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
103-103: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
114-114: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (11)
docs/source/conf.py (2)
13-16: Updated metadata reflects new project details and version correctly.
24-30: The updated path setup usingPathfrompathlibenhances readability and maintainability.src/MEDS_tabular_automl/scripts/cache_task.py (1)
13-15: Switching to relative imports enhances modularity and consistency across the project.src/MEDS_tabular_automl/scripts/tabularize_time_series.py (1)
18-23: Adopting relative imports for internal modules is a good practice for ensuring better project structure.Tools
Ruff
18-18: Module level import not at top of file (E402)
19-19: Module level import not at top of file (E402)
20-20: Module level import not at top of file (E402)
21-21: Module level import not at top of file (E402)
22-22: Module level import not at top of file (E402)
src/MEDS_tabular_automl/scripts/tabularize_static.py (1)
17-26: The use of relative imports for internal modules is consistent with best practices and improves maintainability.Tools
Ruff
17-22: Module level import not at top of file (E402)
23-23: Module level import not at top of file (E402)
24-24: Module level import not at top of file (E402)
25-25: Module level import not at top of file (E402)
README.md (4)
49-49: Reorganization of the Scripts and Examples section enhances clarity and accessibility.
57-57: The description of core CLI scripts is clear and well-detailed.
127-127: The addition of the "Additional CLI Scripts" section is a useful resource for users, providing practical examples of how to use thegenerate-permutationscommand.
[APROVED]
152-152: The explanation under "What do you mean 'tabular pipelines'?" provides a good clarification on the misconception about structured EHR data.src/MEDS_tabular_automl/utils.py (1)
81-81: Addingreplace=TruetoOmegaConf.register_new_resolverensures that the resolver can be redefined without a conflict, which is a good practice when configurations might be dynamically reloaded or tested repeatedly.src/MEDS_tabular_automl/scripts/launch_xgboost.py (1)
16-18: Switching to relative imports enhances modularity and makes the codebase more maintainable by reducing dependency on the absolute structure of the project.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/source/conf.py (3 hunks)
Additional context used
Ruff
docs/source/conf.py
23-23: Module level import not at top of file (E402)
56-56: Module level import not at top of file (E402)
Additional comments not posted (3)
docs/source/conf.py (3)
14-17: Updated project information looks accurate and reflects the new version and additional authors.
25-31: The use ofPathfor directory manipulations enhances readability and reliability of the path setup.
51-51: Sphinx extension configurations are correctly set up.
|
|
||
| # TODO: use https://github.com/sphinx-extensions2/sphinx-autodoc2 | ||
|
|
||
| from sphinx.ext import apidoc |
There was a problem hiding this comment.
Move the apidoc import to the top of the file to adhere to PEP 8 guidelines.
- from sphinx.ext import apidoc
+ import os
+ import sys
+ from sphinx.ext import apidocCommittable suggestion was skipped due to low confidence.
Tools
Ruff
56-56: Module level import not at top of file (E402)
| try: | ||
| cmd_line = f"--implicit-namespaces -e -f -o {output_dir} {module_dir}" | ||
| apidoc.main(cmd_line.split(" ")) | ||
| except Exception as e: # pylint: disable=broad-except | ||
| print(f"Running `sphinx-apidoc {cmd_line}` failed!\n{e}") |
There was a problem hiding this comment.
Consider refining the exception handling to catch more specific exceptions rather than a broad catch. This will help in diagnosing specific issues more effectively.
- except Exception as e: # pylint: disable=broad-except
+ except (IOError, OSError) as e:Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| cmd_line = f"--implicit-namespaces -e -f -o {output_dir} {module_dir}" | |
| apidoc.main(cmd_line.split(" ")) | |
| except Exception as e: # pylint: disable=broad-except | |
| print(f"Running `sphinx-apidoc {cmd_line}` failed!\n{e}") | |
| try: | |
| cmd_line = f"--implicit-namespaces -e -f -o {output_dir} {module_dir}" | |
| apidoc.main(cmd_line.split(" ")) | |
| except (IOError, OSError) as e: | |
| print(f"Running `sphinx-apidoc {cmd_line}` failed!\n{e}") |
|
|
||
| # -- General configuration --------------------------------------------------- | ||
| # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration | ||
|
|
||
| sys.path.insert(0, os.path.abspath("../..")) | ||
| # -- Path setup | ||
| from pathlib import Path |
There was a problem hiding this comment.
Consider moving the Path import to the top to adhere to PEP 8 guidelines.
- from pathlib import Path
+ import os
+ import sys
+ from pathlib import PathCommittable suggestion was skipped due to low confidence.
Tools
Ruff
23-23: Module level import not at top of file (E402)
Summary by CodeRabbit
Documentation
Chores
docs/source/generateddirectory to.gitignore.Refactor
Release Management