explicit update() method for query_params

[Asaurus1]

Describe your changes

Adds an explicit .update() method to st.query_params

Other changes

• Exclude "e2e_flaky" folder from pre-commit formatting checks since the symlinks there cause problems for contributors building on Windows, and on Linux there no point in checking the formatting of a symlinked file that gets covered anyways
• Changes the EXCLUDE list of file paths in "scripts/mypy" to use os.path.join instead of hard coded "/" so that, again, path matching works on systems which use "\" as a path separator.
• Adds a __len__ method to ForwardMsgQueue because it makes obvious sense and aids with the new testing added on this PR.

GitHub Issue Link (if applicable)

#8199

Testing Plan

Unit tests updated

---

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

[sfc-gh-jcarroll]

Awesome thanks @Asaurus1 !

[sfc-gh-wihuang]

I think most things here look good to me. If you could, could you fix the github actions? You should be able to run
make pyformat from the root directory to fix the problems.

[Asaurus1]

I think most things here look good to me. If you could, could you fix the github actions? You should be able to run make pyformat from the root directory to fix the problems.

Yep will implement all these. I gotta do some fancy stuff on windows for pyformat because black does not like the symlink files but it'll work out.

[Asaurus1]

@sfc-gh-wihuang all updated

[Asaurus1]

@sfc-gh-wihuang I'd like to add a second function, .pop_list(keys: list[str]) which atomically removes a list of keys from the query_params dict. This doesn't necessarily solve the original problem from the bug report, but it does allow advanced users (such as myself) to remove a bunch of values from query_params without sending a bunch of ForwardMsg's.

Alternatively, a .set_dict(dict) method which would behave like a call to .clear(); .update(dict) without the intermediate "flash" that would occur with both calls being separate.

Thoughts?

[sfc-gh-jcarroll]

pop_list() / set_dict() should be in a separate PR, want to think about that a little more on the product side whereas this one should be good to go once code review and tests are green. Unless you want to block this one on resolving that too.

Might be worth making a small feature enhancement for that, describe the use case a little and just tag me before spending time writing the PR. I think some version of this probably does make sense but want to think through it.

[Asaurus1]

pop_list() / set_dict() should be in a separate PR, want to think about that a little more on the product side whereas this one should be good to go once code review and tests are green. Unless you want to block this one on resolving that too.

Might be worth making a small feature enhancement for that, describe the use case a little and just tag me before spending time writing the PR. I think some version of this probably does make sense but want to think through it.

Can be a separate PR, sure. Don't hold this one up for it. 🙂

[sfc-gh-wihuang]

LGTM! Thanks @Asaurus1 !

[Asaurus1]

@willhuang1997 I don't think this licensing jstest failure is due to any changes I made, but let me know if I need to fix something there.

[willhuang1997]

hi @Asaurus1 , this is something the team is currently looking into and not your problem. Also, are these codeql notices something to worry about? It seems like we are overloading methods to deal with python types if I understand?

[Asaurus1]

hi @Asaurus1 , this is something the team is currently looking into and not your problem. Also, are these codeql notices something to worry about? It seems like we are overloading methods to deal with python types if I understand?

Yeah, the codeql doesn't seem to understand the point of the @overload decorator for type checking, but we use @overload plenty of other places in the library in the same way.

[LukasMasuch]

@Asaurus1 Yep, CodeQL gets the overload wrong :( I dismissed all of those as False positive.

[Asaurus1]

@willhuang1997 do I have any further actions here? I think all questions so far have been addressed.

[sfc-gh-wihuang]

@willhuang1997 do I have any further actions here? I think all questions so far have been addressed.

Hi, can you merge develop in? That should be the last step.

Thanks!

[Asaurus1]

@willhuang1997 do I have any further actions here? I think all questions so far have been addressed.

Hi, can you merge develop in? That should be the last step.

Thanks!

Yep! Squashed and rebased onto develop.

As I was going through, I did notice an unrelated thing. The pre-commit file uses a specific, pinned version of black (https://github.com/streamlit/streamlit/blob/develop/.pre-commit-config.yaml#L23) but the dev-requirements.txt file calls out a back-pinned version of black only (https://github.com/streamlit/streamlit/blob/develop/lib/dev-requirements.txt#L1); since the Makefile has commands that both run black directly and calls out pre-commit run black in some places, should we be making sure that the two checks are using the same version?

[willhuang1997]

@Asaurus1 , I think they should be the same version but that's probably a problem for us at streamlit to figure out.

[willhuang1997]

Also, feel free to merge. Otherwise, I can merge.

[Asaurus1]

Also, feel free to merge. Otherwise, I can merge.

I do not have merge permissions yet 🫠.