Skip to content
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

Allow default_ignore_list to be empty #540

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

encukou
Copy link
Contributor

@encukou encukou commented Apr 9, 2024

Hello,
Thank you for freezegun!

I wanted to disable the default ignore list, but I noticed that configure(default_ignore_list=[]) is equivalent to default_ignore_list=None. I believe it should instead ignore nothing.

I added a is not None, and a test.
I added the same test for test_extend_default_ignore_list, but since None and [] are equivalent there, that doesn't need code changes.

Previously, `configure(default_ignore_list=[])` was equivalent to
default_ignore_list=None. It should instead set the ignore list
to nothing.
Copy link
Contributor

@kingbuzzman kingbuzzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but i'd get rid of these comments, they add no value whatsoever. 2 cents.

    ['threading', 'tensorflow'],  # example from docs
    [],  # ignore nothing
    ['tensorflow'],  # example from docs
    [],  # ignore nothing extra

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense @encukou - thanks for fixing this!

@bblommers bblommers merged commit 4ecad0c into spulec:master Apr 22, 2024
10 checks passed
@encukou encukou deleted the fix-empty-ignore branch April 23, 2024 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants