Skip to content

Comments

Remove exessive imports#30404

Closed
agalst wants to merge 5 commits intoapache:mainfrom
agalst:agalsty/removegeneralimports
Closed

Remove exessive imports#30404
agalst wants to merge 5 commits intoapache:mainfrom
agalst:agalsty/removegeneralimports

Conversation

@agalst
Copy link

@agalst agalst commented Mar 31, 2023

Remove unnecessary general imports for performance improvement


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:CLI area:core-operators provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:logging area:providers area:Scheduler including HA (high availability) scheduler area:serialization provider:amazon AWS/Amazon - related issues provider:google Google (including GCP) related issues provider:trino labels Mar 31, 2023
@agalst agalst changed the title Agalsty/removegeneralimports Remove whole package imports Mar 31, 2023
@agalst agalst changed the title Remove whole package imports Remove exessive imports Mar 31, 2023

from http import HTTPStatus

import pendulum
Copy link
Contributor

Choose a reason for hiding this comment

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

The pendulum module must be loaded in either case here. I don't think you will see a performance gain from a change like this.

I ran a very quick couple of commands to test this:
Screenshot from 2023-03-31 13-20-06

You can see that they're the same (even the from case being slower, but this is normal variance of course).

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, speed-wise seems there is no real difference. However, importing the whole pandas namespace to use only DataFrame seems excessive. What do you think @o-nikolas ?

Copy link
Member

Choose a reason for hiding this comment

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

That’s not how Python works. import pandas and from pandas import DataFrame has exactly the same import time.

@uranusjr
Copy link
Member

uranusjr commented Apr 3, 2023

Sorry for being blunt, but this is useless.

@uranusjr uranusjr closed this Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:CLI area:logging area:providers area:Scheduler including HA (high availability) scheduler area:serialization provider:amazon AWS/Amazon - related issues provider:cncf-kubernetes Kubernetes (k8s) provider related issues provider:google Google (including GCP) related issues provider:trino

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants