-
Notifications
You must be signed in to change notification settings - Fork 1
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 to Module API #59
Conversation
Warning Rate limit exceeded@justin13601 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 7 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent documentation updates focus on refining type declarations and references within the documentation of 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
docs/source/usage.md (2)
Line range hint
178-178
: Correct grammatical error in the documentation.- `cohort_dir`: Directory the your task configuration file + `cohort_dir`: Directory of your task configuration file
Line range hint
70-70
: Add a language specifier to the fenced code block.- ```bash + ```yaml
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/source/usage.md (1 hunks)
Additional context used
LanguageTool
docs/source/usage.md
[uncategorized] ~64-~64: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... the timestamp at which a prediction is made and can be set tostart
orend
of a...
[uncategorized] ~154-~154: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...To set a data standard:data.standard
: String specifying the data standard, mu...
[uncategorized] ~158-~158: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...y from a single MEDS shard:data.path
: Path to the.parquet
shard file To qu...
[uncategorized] ~162-~162: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ata=sharded. Additionally:
data.root`: Root directory of MEDS dataset containi...
[uncategorized] ~164-~164: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ntaining shard directoriesdata.shard
: Expression specifying MEDS shards (`$(e...
[uncategorized] ~168-~168: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...uery from an ESGPT dataset:data.path
: Directory of the full ESGPT dataset To...
[uncategorized] ~174-~174: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...e predicates dataframedata.ts_format
: Timestamp format for predicates. Defaul...
[uncategorized] ~178-~178: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ..." #### Task Configurationcohort_dir
: Directory the your task configuration f...
[grammar] ~178-~178: A determiner cannot be combined with a possessive pronoun. Did you mean simply “the” or “your”? (A_MY)
Context: ... Configurationcohort_dir
: Directory the your task configuration filecohort_name
:...
[uncategorized] ~180-~180: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...r task configuration filecohort_name
: Name of the task configuration file Th...
[uncategorized] ~184-~184: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ing results, and logging:config_path
: Path to the task configuration file. De...
[uncategorized] ~186-~186: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: .../${cohort_name}.yamloutput_filepath`: Path to store the outputs. Defaults to ...
Markdownlint
docs/source/usage.md
70-70: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
docs/source/usage.md
Outdated
@@ -237,7 +237,7 @@ You can also use the `aces.query.query()` function to extract a cohort in Python | |||
.. autofunction:: aces.query.query | |||
``` | |||
|
|||
The `cfg` parameter must be of type `config.TaskExtractorConfig`, and the `predicates_df` parameter must be of type `polars.DataFrame`. | |||
The `cfg` parameter must be of type :py:mod:`aces.config.TaskExtractorConfig`, and the `predicates_df` parameter must be of type `polars.DataFrame`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the type reference to match the PR objectives.
The PR aims to update the type declaration of the cfg
parameter in the aces.query.query()
function to :py:mod:aces.config.TaskExtractorConfig
. However, the current documentation does not reflect this change accurately. Please update it to match the intended type reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- docs/source/algorithm.md (1 hunks)
- docs/source/configuration.md (6 hunks)
- docs/source/technical.md (1 hunks)
- docs/source/usage.md (3 hunks)
Files skipped from review due to trivial changes (1)
- docs/source/technical.md
Additional context used
LanguageTool
docs/source/usage.md
[uncategorized] ~64-~64: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ... the timestamp at which a prediction is made and can be set tostart
orend
of a...
[uncategorized] ~139-~139: The preposition “on” seems more likely in this position than the preposition “in”. (AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_ON)
Context: ...e overridden by specifying their values in the command-line. #### Data Configurat...
[uncategorized] ~145-~145: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...To set a data standard:data.standard
: String specifying the data standard, mu...
[uncategorized] ~149-~149: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...y from a single MEDS shard:data.path
: Path to the.parquet
shard file To qu...
[uncategorized] ~153-~153: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ata=sharded. Additionally:
data.root`: Root directory of MEDS dataset containi...
[uncategorized] ~155-~155: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ntaining shard directoriesdata.shard
: Expression specifying MEDS shards (`$(e...
[uncategorized] ~159-~159: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...uery from an ESGPT dataset:data.path
: Directory of the full ESGPT dataset To...
[uncategorized] ~165-~165: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...e predicates dataframedata.ts_format
: Timestamp format for predicates. Defaul...
[uncategorized] ~169-~169: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ..." #### Task Configurationcohort_dir
: Directory of your task configuration fi...
[uncategorized] ~171-~171: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...r task configuration filecohort_name
: Name of the task configuration file Th...
[uncategorized] ~175-~175: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ing results, and logging:config_path
: Path to the task configuration file. De...
[uncategorized] ~177-~177: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: .../${cohort_name}.yamloutput_filepath`: Path to store the outputs. Defaults to ...
[uncategorized] ~193-~193: This verb may not be in the correct form. Consider using a different form for this context. (AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
Context: ...files in MEDS. As such, the data can be loading by providing a path pointing to the `.p...docs/source/configuration.md
[uncategorized] ~30-~30: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...consists of three parts: -predicates
, stored as a dictionary from string pred...
[uncategorized] ~33-~33: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...t build on other predicates. -trigger
, stored as a string toEventConfig
- `...
[uncategorized] ~47-~47: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... of the following four fields: -code
: The string value for the categorical co...
[uncategorized] ~49-~49: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...s code in the observation. -value_min
: If specified, an observation will only ...
[style] ~51-~51: ‘on the basis of’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_ON_THE_BASIS_OF)
Context: ..._min(with these options being decided on the basis of
value_min_inclusive, where
value_m...
[uncategorized] ~55-~55: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... equal to will be used). -value_max
: If specified, an observation will only ...
[style] ~57-~57: ‘on the basis of’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_ON_THE_BASIS_OF)
Context: ..._max(with these options being decided on the basis of
value_max_inclusive, where
value_m...
[uncategorized] ~61-~61: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... will be used). -value_min_inclusive
: Seevalue_min
-value_max_inclusive
...
[uncategorized] ~95-~95: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... -and(pred_1_name, pred_2_name, ...)
: Asserts that all of the specified predi...
[style] ~95-~95: Consider removing “of” to be more concise (ALL_OF_THE)
Context: ..._name, pred_2_name, ...): Asserts that all of the specified predicates must be true. -
o...
[uncategorized] ~96-~96: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...e. -or(pred_1_name, pred_2_name, ...)
: Asserts that any of the specified predi...
[uncategorized] ~137-~137: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...REFERENCING = $REFERENCED + $TIME_DELTA,
$REFERENCING = $REFERENCED - $TIME_DEL...
[uncategorized] ~146-~146: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...REFERENCING = $REFERENCED -> $PREDICATE,
$REFERENCING = $REFERENCED <- $PREDICA...
[uncategorized] ~181-~181: Possible missing preposition found. (AI_HYDRA_LEO_MISSING_OF)
Context: ...max_valid)` that define the valid range the count of observations of the named pred...docs/source/algorithm.md
[grammar] ~3-~3: The verb after “to” should be in the base form as part of the to-infinitive. A verb can take many forms, but the base form is always used in the to-infinitive. (TO_NON_BASE)
Context: ...ch details events that have happened to subjects. Each row in the dataframe will have a ...
[style] ~8-~8: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it. (A_VARIETY_OF)
Context: ... of columns which describe the event in a variety of ways. These columns can either have a b...
[style] ~43-~43: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it. (A_VARIETY_OF)
Context: ...in this particular case could represent a variety of other events in the medical record, suc...
[grammar] ~91-~91: Did you mean “exists”? (THERE_VBP_A_NN)
Context: ...n a "valid" form for a patient if there exist a set of time points such that, within ...
[uncategorized] ~144-~144: The abbreviation “i.e.” (= that is) requires two periods. (I_E)
Context: ...s strictly increasing in our algorithm (ie., the start of a "window" will always be...
[uncategorized] ~191-~191: This verb may not be in the correct form. Consider using a different form for this context. (AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
Context: ... object containing our task definition, include all information about predicates, the t...
[style] ~205-~205: ‘Prior to’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_PRIOR_TO)
Context: ...#### Identify Prospective Root Anchors Prior to summarizing the rest of the task tree, ...
[style] ~241-~241: ‘In the event that’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
Context: ...ng the recursive step. #####offset
In the event that the subtree root timestamp is not the s...
[grammar] ~244-~244: The modal verb ‘will’ requires the verb’s base form. (MD_BASEFORM)
Context: ... If the two are not the same, they will guaranteed to be separated by a constantoffset
...
[uncategorized] ~257-~257: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...n a dataframe keyed by thesubject_id
column as well as by any possible prospective ...
[uncategorized] ~334-~334: The preposition ‘as’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_TO_AS)
Context: ...his timestamp can be manually specified to any start or end timestamp of any desir...
[uncategorized] ~336-~336: The abbreviation “i.e.” (= that is) requires two periods. (I_E)
Context: ...t which point a prediction can be made (ie., at the end of theinput
windows). #...
Markdownlint
docs/source/algorithm.md
126-126: Expected: h3; Actual: h4 (MD001, heading-increment)
Heading levels should only increment by one level at a time
100-100: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (1)
docs/source/usage.md (1)
231-231
: Correct the type reference for thecfg
parameter in theaces.query.query()
function.- cfg: :py:mod:`aces.config.TaskExtractorConfig` + cfg: {py:class}`aces.config.TaskExtractorConfig`This change aligns with the PR objectives and ensures that the type reference is consistently formatted across the documentation.
Likely invalid or redundant comment.
{py:class}`aces.config.PlainPredicateConfig` objects, which store raw predicates with no dependencies on other predicates, or | ||
{py:class}`aces.config.DerivedPredicateConfig` objects, which store predicates that build on other predicates. | ||
- `trigger`, stored as a string to `EventConfig` | ||
- `windows`, stored as a dictionary from string window names (which must be unique) to `WindowConfig` | ||
- `windows`, stored as a dictionary from string window names (which must be unique) to {py:class}`aces.config.WindowConfig` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update class references to use consistent and correct Sphinx notation.
- {py:class}`aces.config.PlainPredicateConfig`
+ :py:class:`aces.config.PlainPredicateConfig`
- {py:class}`aces.config.DerivedPredicateConfig`
+ :py:class:`aces.config.DerivedPredicateConfig`
- {py:class}`aces.config.WindowConfig`
+ :py:class:`aces.config.WindowConfig`
This change corrects the syntax for referencing Python classes in Sphinx documentation, ensuring that the documentation is properly rendered.
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.
{py:class}`aces.config.PlainPredicateConfig` objects, which store raw predicates with no dependencies on other predicates, or | |
{py:class}`aces.config.DerivedPredicateConfig` objects, which store predicates that build on other predicates. | |
- `trigger`, stored as a string to `EventConfig` | |
- `windows`, stored as a dictionary from string window names (which must be unique) to `WindowConfig` | |
- `windows`, stored as a dictionary from string window names (which must be unique) to {py:class}`aces.config.WindowConfig` | |
:py:class:`aces.config.PlainPredicateConfig` objects, which store raw predicates with no dependencies on other predicates, or | |
:py:class:`aces.config.DerivedPredicateConfig` objects, which store predicates that build on other predicates. | |
- `trigger`, stored as a string to `EventConfig` | |
- `windows`, stored as a dictionary from string window names (which must be unique) to :py:class:`aces.config.WindowConfig` |
Tools
LanguageTool
[uncategorized] ~33-~33: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...t build on other predicates. -trigger
, stored as a string toEventConfig
- `...
@@ -188,7 +188,7 @@ During initialization, we will be given the following inputs: | |||
|
|||
##### `cfg` | |||
|
|||
`cfg` is a `TaskExtractorConfig` object containing our task definition, include all information about | |||
`cfg` is a {py:class}`aces.config.TaskExtractorConfig` object containing our task definition, include all information about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure the class reference uses the correct Sphinx notation.
- cfg is a {py:class}`aces.config.TaskExtractorConfig`
+ cfg is a :py:class:`aces.config.TaskExtractorConfig`
This correction ensures that the class reference is properly formatted for Sphinx documentation, allowing for correct rendering and linking.
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.
`cfg` is a {py:class}`aces.config.TaskExtractorConfig` object containing our task definition, include all information about | |
`cfg` is a :py:class:`aces.config.TaskExtractorConfig` object containing our task definition, include all information about |
Tools
LanguageTool
[uncategorized] ~191-~191: This verb may not be in the correct form. Consider using a different form for this context. (AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
Context: ... object containing our task definition, include all information about predicates, the t...
Summary by CodeRabbit
usage.md
,algorithm.md
, andconfiguration.md
files for enhanced clarity and consistency.terminology.md
withalgorithm.md
intechnical.md
.