Skip to content

[AIRFLOW-4891] Extend list of pylint good-names#5524

Merged
potiuk merged 1 commit intoapache:masterfrom
PolideaInternal:pylint-goodnames
Jul 4, 2019
Merged

[AIRFLOW-4891] Extend list of pylint good-names#5524
potiuk merged 1 commit intoapache:masterfrom
PolideaInternal:pylint-goodnames

Conversation

@turbaszek
Copy link
Member

@turbaszek turbaszek commented Jul 3, 2019

I would like to propose further extension of pylint good-names:

@turbaszek
Copy link
Member Author

@mik-laj @potiuk @ashb what are your thoughts?

@ashb
Copy link
Member

ashb commented Jul 3, 2019

Are there any others do you think? Should we wait/keep this Jira open for a while?

@turbaszek
Copy link
Member Author

Here is a list of top 10 invalid names with number of occurrences:

  12  Variable name "t2" doesn't conform to snake_case naming style (invalid-name)
  13  Variable name "ts" doesn't conform to snake_case naming style (invalid-name)
  15  Variable name "df" doesn't conform to snake_case naming style (invalid-name)
  17  Variable name "v" doesn't conform to snake_case naming style (invalid-name)
  19  Variable name "p" doesn't conform to snake_case naming style (invalid-name)
  22  Variable name "b" doesn't conform to snake_case naming style (invalid-name)
  23  Variable name "m" doesn't conform to snake_case naming style (invalid-name)
  24  Variable name "c" doesn't conform to snake_case naming style (invalid-name)
  26  Variable name "s" doesn't conform to snake_case naming style (invalid-name)
  39  Variable name "d" doesn't conform to snake_case naming style (invalid-name)
 107  Variable name "t" doesn't conform to snake_case naming style (invalid-name)

So I would suggest also df as it's popular name for pd.DataFrame.

@ashb
Copy link
Member

ashb commented Jul 3, 2019

Yes to df. ts = timestamp?

What are the uses of t? That's probably task isn't it? It may actually be clearer to use task instead of t. Not sure why I feel different about that var name... :D

@turbaszek
Copy link
Member Author

turbaszek commented Jul 3, 2019

From macro documentation:

{{ ts }} | same as execution_date.isoformat(). Example: 2018-01-01T00:00:00+00:00

You are right that t is often used in tests for task / sensor / operator. But there are also other usages like t = MySQLdb.constants.FIELD_TYPE so I would consider t too ambiguous.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

OK. Also I think df is the only from the list that should be OK

@codecov-io
Copy link

Codecov Report

Merging #5524 into master will increase coverage by 0.24%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5524      +/-   ##
==========================================
+ Coverage   78.59%   78.83%   +0.24%     
==========================================
  Files         489      489              
  Lines       30744    30744              
==========================================
+ Hits        24163    24237      +74     
+ Misses       6581     6507      -74
Impacted Files Coverage Δ
airflow/operators/postgres_operator.py 0% <0%> (-100%) ⬇️
airflow/executors/sequential_executor.py 47.61% <0%> (-52.39%) ⬇️
airflow/utils/sqlalchemy.py 73.25% <0%> (-5.82%) ⬇️
airflow/executors/__init__.py 62.5% <0%> (-4.17%) ⬇️
airflow/hooks/postgres_hook.py 94.64% <0%> (-1.79%) ⬇️
airflow/hooks/dbapi_hook.py 86.84% <0%> (-1.76%) ⬇️
airflow/jobs/scheduler_job.py 68.57% <0%> (-1.71%) ⬇️
airflow/utils/dag_processing.py 59.24% <0%> (-1.03%) ⬇️
airflow/www/views.py 74.47% <0%> (-0.84%) ⬇️
airflow/models/taskinstance.py 92.69% <0%> (-0.5%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75872a2...5218946. Read the comment docs.

@potiuk potiuk merged commit dc6909f into apache:master Jul 4, 2019
andriisoldatenko pushed a commit to andriisoldatenko/airflow that referenced this pull request Jul 26, 2019
wmorris75 pushed a commit to modmed-external/incubator-airflow that referenced this pull request Jul 29, 2019
@turbaszek turbaszek deleted the pylint-goodnames branch September 19, 2019 11:56
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.

4 participants