-
Notifications
You must be signed in to change notification settings - Fork 7
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
Feat: introduce streamlit integration #68
Conversation
WalkthroughThe updates encompass a broad restructuring and enhancement of the RxDB integration within Angular and Streamlit environments. Key changes include renaming classes and services for consistency, refining RxDB configuration and service implementations, and introducing new functionalities like managing RxDB dataframes in Streamlit. The modifications streamline development workflows, improve code readability, and expand the capabilities of applications using RxDB for database management. Changes
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 (
|
packages/streamlit-rxdb-dataframe/frontend/build/package.json.old
Outdated
Show resolved
Hide resolved
packages/streamlit-rxdb-dataframe/frontend/src/lib/useEditingState.ts
Outdated
Show resolved
Hide resolved
packages/streamlit-rxdb-dataframe/frontend/src/lib/RxDBDataframe.tsx
Outdated
Show resolved
Hide resolved
63bb1fc
to
e237a20
Compare
packages/streamlit-rxdb-dataframe/rxdb_dataframe/frontend/src/lib/RxDBDataframe.tsx
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
plugins: [ | ||
// will be loaded by together with core plugins | ||
RxDBDevModePlugin, // <- add only for development | ||
RxDBAttachmentsPlugin, | ||
RxDBLeaderElectionPlugin, | ||
], |
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.
Adding new plugins (RxDBDevModePlugin
, RxDBAttachmentsPlugin
, RxDBLeaderElectionPlugin
) enhances the functionality significantly. However, it's crucial to document the specific use cases and benefits of each plugin within the README to help users understand their purpose and how they can leverage them in their projects.
Would you like assistance in drafting brief descriptions for these plugins?
plugins: [ | ||
// will be loaded by together with core plugins | ||
RxDBDevModePlugin, // <- add only for development | ||
RxDBAttachmentsPlugin, | ||
RxDBLeaderElectionPlugin, | ||
], |
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.
Repeating the addition of new plugins (RxDBDevModePlugin
, RxDBAttachmentsPlugin
, RxDBLeaderElectionPlugin
) in the standalone configuration is consistent with the NgModule configuration. This ensures that the enhanced functionality is available across different types of application setups. Just like before, adding descriptions for these plugins in the documentation would be beneficial for users.
Would you like help with adding these plugin descriptions to the standalone section as well?
- Persist collection query ([mango-query-syntax](https://github.com/cloudant/mango)) in URL with new plugin `query-params-plugin` (in demo, set localStorage `_ngx_rxdb_queryparams` ) | ||
- provide Observable of current URL (automatically for Angular) | ||
- simple methods to set or patch filter, sort, limit, skip |
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.
The introduction of the query-params-plugin
for persisting collection queries in the URL is an innovative feature that can enhance user experience by maintaining state across sessions. It's important to provide examples and documentation on how to use this plugin effectively, including how to set up and retrieve the persisted queries.
If needed, I can contribute examples and documentation for the query-params-plugin
. Would you like me to do so?
def test_get_dataframe_by_schema(): | ||
# Test case 1: Empty schema | ||
schema = {} | ||
expected_df = pd.DataFrame() | ||
assert get_dataframe_by_schema(schema).equals(expected_df) | ||
|
||
# Test case 2: Schema with string properties | ||
schema = { | ||
"properties": { | ||
"name": {"type": "string"}, | ||
"age": {"type": "string"}, | ||
"city": {"type": "string"}, | ||
} | ||
} | ||
expected_df = pd.DataFrame( | ||
{ | ||
"name": pd.Series(dtype="object"), | ||
"age": pd.Series(dtype="object"), | ||
"city": pd.Series(dtype="object"), | ||
} | ||
) | ||
assert get_dataframe_by_schema(schema).equals(expected_df) | ||
|
||
# Test case 3: Schema with boolean properties | ||
schema = { | ||
"properties": {"is_active": {"type": "boolean"}, "has_permission": {"type": "boolean"}} | ||
} | ||
expected_df = pd.DataFrame( | ||
{"is_active": pd.Series(dtype="bool"), "has_permission": pd.Series(dtype="bool")} | ||
) | ||
assert get_dataframe_by_schema(schema).equals(expected_df) | ||
|
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.
The test_get_dataframe_by_schema
function is duplicated, which will result in only the latter being executed. Consider merging these tests into a single function or renaming one to ensure all intended tests are run.
- def test_get_dataframe_by_schema():
+ def test_get_dataframe_by_schema_additional_cases():
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.
def test_get_dataframe_by_schema(): | |
# Test case 1: Empty schema | |
schema = {} | |
expected_df = pd.DataFrame() | |
assert get_dataframe_by_schema(schema).equals(expected_df) | |
# Test case 2: Schema with string properties | |
schema = { | |
"properties": { | |
"name": {"type": "string"}, | |
"age": {"type": "string"}, | |
"city": {"type": "string"}, | |
} | |
} | |
expected_df = pd.DataFrame( | |
{ | |
"name": pd.Series(dtype="object"), | |
"age": pd.Series(dtype="object"), | |
"city": pd.Series(dtype="object"), | |
} | |
) | |
assert get_dataframe_by_schema(schema).equals(expected_df) | |
# Test case 3: Schema with boolean properties | |
schema = { | |
"properties": {"is_active": {"type": "boolean"}, "has_permission": {"type": "boolean"}} | |
} | |
expected_df = pd.DataFrame( | |
{"is_active": pd.Series(dtype="bool"), "has_permission": pd.Series(dtype="bool")} | |
) | |
assert get_dataframe_by_schema(schema).equals(expected_df) | |
def test_get_dataframe_by_schema_additional_cases(): | |
# Test case 1: Empty schema | |
schema = {} | |
expected_df = pd.DataFrame() | |
assert get_dataframe_by_schema(schema).equals(expected_df) | |
# Test case 2: Schema with string properties | |
schema = { | |
"properties": { | |
"name": {"type": "string"}, | |
"age": {"type": "string"}, | |
"city": {"type": "string"}, | |
} | |
} | |
expected_df = pd.DataFrame( | |
{ | |
"name": pd.Series(dtype="object"), | |
"age": pd.Series(dtype="object"), | |
"city": pd.Series(dtype="object"), | |
} | |
) | |
assert get_dataframe_by_schema(schema).equals(expected_df) | |
# Test case 3: Schema with boolean properties | |
schema = { | |
"properties": {"is_active": {"type": "boolean"}, "has_permission": {"type": "boolean"}} | |
} | |
expected_df = pd.DataFrame( | |
{"is_active": pd.Series(dtype="bool"), "has_permission": pd.Series(dtype="bool")} | |
) | |
assert get_dataframe_by_schema(schema).equals(expected_df) | |
provide: RXDB_COLLECTION, | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore // INFO: no need for typings here, nothing's exposed, but ts complains // NOSONAR | ||
useFactory: (config, dbService, ngZone, currentUrl, updateQueryParamsFn) => | ||
new RxDBCollectionService( | ||
config, | ||
dbService, | ||
ngZone, | ||
currentUrl, | ||
updateQueryParamsFn | ||
), | ||
deps: [ | ||
// | ||
RXDB_CONFIG_COLLECTION, | ||
RXDB, | ||
NgZone, | ||
CURRENT_URL, | ||
updateQueryParams, | ||
], |
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.
The introduction of new injection tokens and refactoring align with improving modularity and maintainability. However, the use of // @ts-ignore
should be addressed to ensure type safety. Consider resolving the underlying TypeScript type issue directly.
- // @ts-ignore // INFO: no need for typings here, nothing's exposed, but ts complains // NOSONAR
+ // Consider resolving the type issue here instead of using @ts-ignore to ensure type safety.
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.
provide: RXDB_COLLECTION, | |
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | |
// @ts-ignore // INFO: no need for typings here, nothing's exposed, but ts complains // NOSONAR | |
useFactory: (config, dbService, ngZone, currentUrl, updateQueryParamsFn) => | |
new RxDBCollectionService( | |
config, | |
dbService, | |
ngZone, | |
currentUrl, | |
updateQueryParamsFn | |
), | |
deps: [ | |
// | |
RXDB_CONFIG_COLLECTION, | |
RXDB, | |
NgZone, | |
CURRENT_URL, | |
updateQueryParams, | |
], | |
provide: RXDB_COLLECTION, | |
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | |
// Consider resolving the type issue here instead of using @ts-ignore to ensure type safety. | |
useFactory: (config, dbService, ngZone, currentUrl, updateQueryParamsFn) => | |
new RxDBCollectionService( | |
config, | |
dbService, | |
ngZone, | |
currentUrl, | |
updateQueryParamsFn | |
), | |
deps: [ | |
// | |
RXDB_CONFIG_COLLECTION, | |
RXDB, | |
NgZone, | |
CURRENT_URL, | |
updateQueryParams, | |
], |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
The PR introduces Streamlit integration into the project, aiming to enhance development and extend capabilities with new features. Key changes include:
dataframe
in StreamlitThis PR represents a significant effort to improve interactivity, project structure, and functionality, aiming for a more intuitive and efficient development experience.
Does this PR introduce a breaking change?
RxDBDevModePlugin
from auto-loaded plugins, add only on app levelSummary by CodeRabbit
RxDBCollectionService
for improved consistency and performance.