-
Notifications
You must be signed in to change notification settings - Fork 4k
Add default hash func for regex patterns #1232
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
Conversation
tvst
left a comment
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.
Approved! But please resolve comments.
| self.update(h, obj.tell()) | ||
| return h.digest() | ||
|
|
||
| elif isinstance(obj, Pattern): |
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.
Huh, I didn't think this would work... I thought the typing module was supposed to be used only for type hints.
But either way, I think it would be clearer to say isinstance(obj, re.Pattern) instead.
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.
Tried something like that initially but comparing to typing.Pattern seems to be the recommended way to go about things.
>>> re.Pattern
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: module 're' has no attribute 'Pattern'
lib/streamlit/hashing.py
Outdated
| h = hashlib.new("md5") | ||
| self.update(h, obj.pattern) | ||
| self.update(h, obj.flags) | ||
| return h.digest() |
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.
You can just return b"%s:%s" % (obj.pattern, obj.flags) here since it gets md5'ed inside update()
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.
Perhaps it should be return self.to_bytes([obj.pattern, obj.flags])?
Your suggestion also works but we only use %s:%s when we prefix with the type at the moment. The current pattern is to recurse into to_bytes with some more primitive data.
* develop: Release 0.57.1 - Fixes SessionInfo alert bug (#1270) Py2k Elimination (phase 2): Scrubbing all the py2/3 compatibility clauses (#1177) Changing selectbox filtering as case insensitive (#1269) Moving st_in_cache_warning from flaky to stable (#1267) DataFrame | Fixing column width behavior (#1258) Release 0.57 (#1266) Fix bug where script was executing twice on first run. (#1263) Add default hash func for regex patterns (#1232) Updated readme.md help people find msft installation instructions. MediaFileManager: fix "Content-Type" header
* feature/plugins: Release 0.57.1 - Fixes SessionInfo alert bug (streamlit#1270) Py2k Elimination (phase 2): Scrubbing all the py2/3 compatibility clauses (streamlit#1177) Changing selectbox filtering as case insensitive (streamlit#1269) Moving st_in_cache_warning from flaky to stable (streamlit#1267) DataFrame | Fixing column width behavior (streamlit#1258) Release 0.57 (streamlit#1266) Fix bug where script was executing twice on first run. (streamlit#1263) Add default hash func for regex patterns (streamlit#1232) Updated readme.md help people find msft installation instructions. MediaFileManager: fix "Content-Type" header
Issue #1189