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

pycln-fix #396

Merged
merged 2 commits into from
Jun 12, 2023
Merged

pycln-fix #396

merged 2 commits into from
Jun 12, 2023

Conversation

rodolfocarobene
Copy link
Contributor

@rodolfocarobene rodolfocarobene commented Jun 12, 2023

Adding --all parameter to pycln.
See that I had to add an exception for pint-pandas in data.py, one of the unused/used imports...

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Merging #396 (0b72989) into main (cae85be) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #396      +/-   ##
==========================================
- Coverage   95.44%   95.44%   -0.01%     
==========================================
  Files          46       46              
  Lines        2942     2940       -2     
==========================================
- Hits         2808     2806       -2     
  Misses        134      134              
Flag Coverage Δ
unittests 95.44% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/qibocal/auto/task.py 97.61% <ø> (-0.03%) ⬇️
...c/qibocal/protocols/characterization/rabi/utils.py 100.00% <ø> (ø)
src/qibocal/data.py 99.30% <100.00%> (ø)

@@ -5,7 +5,7 @@

import numpy as np
import pandas as pd
import pint_pandas
import pint_pandas # nopycln: import
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I remove that import, several tests fail (I suppose every tests that uses this file).
I am not sure why, but I think that it's implicitly using it with

self._df = pd.DataFrame(
{
"MSR": pd.Series(dtype="pint[V]"),
"i": pd.Series(dtype="pint[V]"),
"q": pd.Series(dtype="pint[V]"),
"phase": pd.Series(dtype="pint[rad]"),
}

Copy link
Member

Choose a reason for hiding this comment

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

Here our first side effect. Most likely pint_pandas is patching pandas (i.e. replacing some content of the module) upon import.

Luckily, we have #372 :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed this is just how pint works for some reason...

Copy link
Member

Choose a reason for hiding this comment

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

It wants to be a drop-in replacement of Pandas, that's the reason. But, yeah, I don't like it so much...

Copy link
Contributor

@andrea-pasquale andrea-pasquale left a comment

Choose a reason for hiding this comment

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

@andrea-pasquale andrea-pasquale added this pull request to the merge queue Jun 12, 2023
Merged via the queue into main with commit e27f72c Jun 12, 2023
@andrea-pasquale andrea-pasquale deleted the pycln-fix branch June 12, 2023 15:29
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.

3 participants