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

web.Application object have a boolean value of False #4102

Closed
Ricordel opened this issue Sep 26, 2019 · 7 comments
Closed

web.Application object have a boolean value of False #4102

Ricordel opened this issue Sep 26, 2019 · 7 comments
Labels
good first issue Good for newcomers Hacktoberfest We think it's good for https://hacktoberfest.digitalocean.com/

Comments

@Ricordel
Copy link

Long story short

Not a huge issue but a bit unsettling: the boolean value of web.Application() is False

Expected behaviour

import aiohttp.web
app = aiohttp.web.Application()
if app:
    print("true")
else:
    print("false")

should print true

Actual behaviour

Prints false

Steps to reproduce

Create an app like in the above code

Your environment

OS: Linux Ubuntu 19.04
python: Python 3.7.3
aiohttp and deps:

aiohttp==3.6.1
async-timeout==3.0.1
attrs==19.1.0
chardet==3.0.4
idna==2.8
multidict==4.5.2
pkg-resources==0.0.0
yarl==1.3.0
@asvetlov
Copy link
Member

Why do you think so?

@Ricordel
Copy link
Author

Ricordel commented Sep 26, 2019

Because it seems to me that a boolean value of False should represent something that is nothing (None), logically or historically false (False, 0) or empty ("", [], {}). I don't think that a constructed and functional web.Application falls into any of those categories.

Also from experience, it seems to me that any random object from any random library has a boolean value of True unless there is a good reason not to (there may be one here I'm not aware of), so this goes against the principle of least astonishment. Python code like:

if subapp:
    app.add_subapp('/foo', app)

or

app = this_user_supplied_app or this_backup_app

will behave in a way that is unexpected for the random Python developer (which I am, I got bitten by the first one).

@asvetlov
Copy link
Member

app is a mapping BTW (app["key"] = value). The idiomatic code should check for None:

if subapp is not None:
    app.add_subapp('/foo', app)

@Ricordel
Copy link
Author

Yes I could figure that out.

I still find the behavior counter-intuitive compared to what all the rest of Python is usually doing, my arguments are in the previous comment. You are free to disagree, and considering your contributions to this project (which I thank you for) you are naturally the one with the final word, and I won't argue (especially since it's not a big problem and it's not preventing anyone to use aiohttp for any reasonable reason).

@asvetlov
Copy link
Member

Ok. I forgot that we had the same problem for exceptions.

Would you want to make a pull request for web.BaseRequest and web.Application? The change can look like here.

A couple of trivial tests are required.

@Ricordel
Copy link
Author

Ok, I'll try to do that in the coming days.

@asvetlov asvetlov added good first issue Good for newcomers Hacktoberfest We think it's good for https://hacktoberfest.digitalocean.com/ labels Oct 1, 2019
@asvetlov
Copy link
Member

asvetlov commented Oct 3, 2019

Fixed by #4124

@asvetlov asvetlov closed this as completed Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Hacktoberfest We think it's good for https://hacktoberfest.digitalocean.com/
Projects
None yet
Development

No branches or pull requests

2 participants