overuse of "pass" keyword? (AKA "pass with care")

[sloanlance]

ArcRest or ArcRestHelper

ArcRest

Version or date of download

Current master branch of this repo

Bug or Enhancement

🐛 Bug ➡️ Enhancement

Repo Steps or Enhancement details

ArcREST contains an overuse of the pass keyword. In some cases, using pass makes sense, especially when declaring a class that inherits everything or needs to be empty. However, many other cases are unnecessary or allow confusing logic.

The following lines of code are overuses of the pass keyword. There may be more, but these are the ones I noticed. In some cases, it can easily be removed without repercussions. (E.g., the last else: pass clauses of conditionals.) In other cases, logic needs to be reversed after removing it. (E.g., it's the then clause, which is followed by an else clause which does something important.)

https://github.com/Esri/ArcREST/blob/master/src/arcrest/agol/services.py#L165 https://github.com/Esri/ArcREST/blob/master/src/arcrest/agol/services.py#L1439 https://github.com/Esri/ArcREST/blob/master/src/arcrest/agol/services.py#L2515 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/_imageservice.py#L148 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/_uploads.py#L45 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/featureservice.py#L57 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/featureservice.py#L160 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/layer.py#L98 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/layer.py#L1157 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/mapservice.py#L74 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/mapservice.py#L223 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/server.py#L88 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/server.py#L222 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/services.py#L57 https://github.com/Esri/ArcREST/blob/master/src/arcrest/ags/services.py#L160 https://github.com/Esri/ArcREST/blob/master/src/arcrest/common/find.py#L134 https://github.com/Esri/ArcREST/blob/master/src/arcrest/common/general.py#L93 https://github.com/Esri/ArcREST/blob/master/src/arcrest/common/general.py#L142 https://github.com/Esri/ArcREST/blob/master/src/arcrest/hostedservice/service.py#L120 https://github.com/Esri/ArcREST/blob/master/src/arcrest/manageags/_clusters.py#L368 https://github.com/Esri/ArcREST/blob/master/src/arcrest/manageags/parameters.py#L114 https://github.com/Esri/ArcREST/blob/master/src/arcrest/manageorg/_parameters.py#L1749 https://github.com/Esri/ArcREST/blob/master/src/arcrest/manageportal/administration.py#L325

The following instance is different, though. Using "pass" in an exception handler has its purposes, but in this case, I think it might not be the right thing to do.

https://github.com/Esri/ArcREST/blob/master/src/arcrest/web/_base.py#L47

[achapkowski]

@sloanlance I don't think this is a bug, but rather an enhancement since the code actually works.

[sloanlance]

That's a stricter criteria for "bug" than what I was using, but I can go along with that. Thanks, @achapkowski.

BTW, I've been trying to figure out whether there's any good reason to use a then or else clause with pass as the only statement. Generally speaking, I can't think of any, but I wonder if there's some Pythonic reason. Do you have any idea?

I've been looking for supporting documentation for or against its use and I've not found very many official answers. I've found only: http://stackoverflow.com/questions/20859998/using-pass-on-a-non-necessary-else-statement

I'm very surprised pylint doesn't think many of these pass statements are "unnecessary". Reviewing the pylint documentation, I see is has a very narrow definition of unnecessary, probably to avoid trouble.