Move check_* utils to own file #8764
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
lib/streamlit/elements/policies.py
Outdated
|
|
||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| import streamlit |
There was a problem hiding this comment.
I'm a bit annoyed by all the import cycle warnings. I believe this import might be the issue. Can we move this import down into the function from where it is used?
There was a problem hiding this comment.
Yeah the circular imports at some places in the code base are pretty annoying. I have spent some time today looking into refactoring it and refactoring everything including the delta generator are on top of my list 😄 moving the imports down - if it works at all as it opened a rabbit hole earlier - just hides the original problem that the code is not pretty well abstracted. I guess eventually we have to start cleaning it up top down.
There was a problem hiding this comment.
moving the imports down - if it works at all as it opened a rabbit hole earlier - just hides the original problem that the code is not pretty well abstracted
Yeah, I think the better solution would be to have some internal import for exception and warning, but I'm not sure if that is viable without bigger refactoring. But just having a blank streamlit import here on top doesn't feel super good. Overall, it's better to move this import down and all the other check_ imports to the top level.
There was a problem hiding this comment.
Since its a delta generator function, the best way would likely be to have to pass the deltagenerator to the function instead of importing a global object. But this surfaces other import issues at other places; so I think the first step has to be abstracting the delta generator and probably move out the main_dg and other *_dg variables and dg_stack out and work with a singleton factory or so. But that requires a little bit more time
raethlein
force-pushed
the
refactor-widget-checks-util
branch
from
73b59b9
to
b5aad90
Compare
raethlein
force-pushed
the
refactor-widget-checks-util
branch
from
94e6571
to
12ec5c3
Compare
|
@LukasMasuch could you please have another look? The CodeScan things look like a lot, but I have moved the imports within the |
raethlein
force-pushed
the
refactor-widget-checks-util
branch
from
12ec5c3
to
18888e0
Compare
Great point, I have moved both utils files |
| from streamlit.elements.lib.policies import ( | ||
| check_cache_replay_rules, | ||
| check_callback_rules, | ||
| check_session_state_rules, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
| from streamlit.elements.lib.policies import ( | ||
| check_cache_replay_rules, | ||
| check_callback_rules, | ||
| check_session_state_rules, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
| from streamlit.elements.lib.policies import ( | ||
| check_cache_replay_rules, | ||
| check_callback_rules, | ||
| check_session_state_rules, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
| from streamlit.elements.lib.policies import ( | ||
| check_cache_replay_rules, | ||
| check_callback_rules, | ||
| check_session_state_rules, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
| from streamlit.elements.lib.policies import ( | ||
| check_cache_replay_rules, | ||
| check_callback_rules, | ||
| check_session_state_rules, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
| from streamlit.elements.lib.policies import ( | ||
| check_cache_replay_rules, | ||
| check_callback_rules, | ||
| check_session_state_rules, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
Describe your changes
In streamlit/elements/utils.py we have some
check_*functions that are used to enforce certain rules. This PR moves them to their ownpolicies.pyfile for cleaner abstraction.Also, tests for
check_cache_replay_ruleswere missing that are added now.This is in preparation of adding a new
check_*policy function in this PR: https://github.com/streamlit/streamlit/pull/8756/filesGitHub Issue Link (if applicable)
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.