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

Do not import pint when it is not needed #1314

Merged
merged 7 commits into from
Feb 7, 2024
Merged

Do not import pint when it is not needed #1314

merged 7 commits into from
Feb 7, 2024

Conversation

jan-janssen
Copy link
Member

No description provided.

@jan-janssen jan-janssen added the format_black reformat the code using the black standard label Feb 4, 2024
@jan-janssen jan-janssen removed the format_black reformat the code using the black standard label Feb 4, 2024
Copy link
Member

@niklassiemer niklassiemer left a comment

Choose a reason for hiding this comment

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

I am ok with the changes in general, especially for such a rare used case the recreation of the function is fine with me. However, I wonder if we could not simply do something like

def _size_conversion(size: "pint.Quantity"):
    import pint
    ...

and leave the function where it was?

@jan-janssen
Copy link
Member Author

I am ok with the changes in general, especially for such a rare used case the recreation of the function is fine with me. However, I wonder if we could not simply do something like

def _size_conversion(size: "pint.Quantity"):
    import pint
    ...

and leave the function where it was?

I do not think that python typing allows defining types as strings.

@jan-janssen jan-janssen added the format_black reformat the code using the black standard label Feb 7, 2024
@jan-janssen
Copy link
Member Author

@niklassiemer I moved the function to a separate module, this way it can still be imported when somebody wants to use it somewhere else but it is not imported by default.

Copy link
Member

@niklassiemer niklassiemer left a comment

Choose a reason for hiding this comment

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

LGTM

@jan-janssen jan-janssen merged commit 416df0b into main Feb 7, 2024
25 checks passed
@jan-janssen jan-janssen deleted the no_pint branch February 7, 2024 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black reformat the code using the black standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants