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

Tests are failing #1005

Closed
Vinc0110 opened this issue Jan 20, 2024 · 11 comments · Fixed by #1006
Closed

Tests are failing #1005

Vinc0110 opened this issue Jan 20, 2024 · 11 comments · Fixed by #1006

Comments

@Vinc0110
Copy link
Collaborator

Pandas 3.0 will be requiring pyarrow:
https://pandas.pydata.org/pandas-docs/version/2.2.0rc0/whatsnew/v2.2.0.html#dedicated-string-data-type-backed-by-arrow-by-default

PyArrow will become a required dependency with pandas 3.0 to accommodate this change.

In #1004 all tests were failing because of various DeprecationWarnings due to this upcoming dependency.

I'm not sure what pandas version is being used in the tests or why this is warning is starting to occur now. Anyway, I've already added pyarrow to pyproject.toml for the test requirements in #1004, which solved the issue, but I thought it would be good to document this change here.

@Vinc0110
Copy link
Collaborator Author

Wow, the tests are still a huge mess:

=========================== short test summary info ============================
  ERROR skrf/calibration/tests/test_calibration.py
  ERROR skrf/calibration/tests/test_calibrationSet.py
  ERROR skrf/calibration/tests/test_deembedding.py
  ERROR skrf/io/tests/test_citi.py
  ERROR skrf/io/tests/test_csv.py
  ERROR skrf/io/tests/test_io.py
  ERROR skrf/io/tests/test_mdif.py
  ERROR skrf/media/tests/test_all_construction.py
  ERROR skrf/media/tests/test_coaxial.py
  ERROR skrf/media/tests/test_cpw.py
  ERROR skrf/media/tests/test_mline.py
  ERROR skrf/tests/test_circuit.py
  ERROR skrf/tests/test_convenience.py
  ERROR skrf/tests/test_frequency.py
  ERROR skrf/tests/test_mathFunctions.py
  ERROR skrf/tests/test_network.py
  ERROR skrf/tests/test_network2.py
  ERROR skrf/tests/test_networkSet.py
  ERROR skrf/tests/test_plotting.py
  ERROR skrf/tests/test_qfactor.py
  ERROR skrf/tests/test_static_data.py
  ERROR skrf/tests/test_tlineFunctions.py
  ERROR skrf/tests/test_util.py
  ERROR skrf/tests/test_vectorfitting.py
  ERROR skrf/vi/tests/test_validators.py
  ERROR skrf/vi/vna/hp/tests/test_8510c.py
  ERROR skrf/vi/vna/keysight/tests/test_fieldfox.py
  ERROR skrf/vi/vna/keysight/tests/test_pna.py
  !!!!!!!!!!!!!!!!!!! Interrupted: 28 errors during collection !!!!!!!!!!!!!!!!!!!
  ============================== 28 errors in 8.99s ==============================
  py39: exit 2 (11.15 seconds) /home/runner/work/scikit-rf/scikit-rf> python -m pytest pid=2004

This does not seem related to my changes. It's more like a general issue.

______________ ERROR collecting skrf/tests/test_vectorfitting.py _______________
  ImportError while importing test module '/home/runner/work/scikit-rf/scikit-rf/skrf/tests/test_vectorfitting.py'.
  Hint: make sure your test modules/packages have valid Python names.
  Traceback:
  /opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/importlib/__init__.py:127: in import_module
      return _bootstrap._gcd_import(name[level:], package, level)
  skrf/tests/test_vectorfitting.py:3: in <module>
      import skrf
  skrf/__init__.py:20: in <module>
      from . import taper
  skrf/taper.py:31: in <module>
      from scipy  import linspace
  E   ImportError: cannot import name 'linspace' from 'scipy' (/home/runner/work/scikit-rf/scikit-rf/.tox/py39/lib/python3.9/site-packages/scipy/__init__.py)

@FranzForstmayr: Any ideas?

@Ttl
Copy link
Collaborator

Ttl commented Jan 21, 2024

It's caused by the new scipy version. linspace should be imported from numpy instead of scipy in taper.py.

@Vinc0110 Vinc0110 linked a pull request Jan 21, 2024 that will close this issue
@Vinc0110
Copy link
Collaborator Author

That was it, thanks!
I did see that error message, but I could not believe this became an issue from one day to another. How is the scipy version changing? Does it depend on some random decision by the github servers?

@Ttl
Copy link
Collaborator

Ttl commented Jan 21, 2024

CI installs the newest packages of dependencies unless specified otherwise. It seems that this was changed in the latest scipy release.

@pjg0707
Copy link

pjg0707 commented Jan 22, 2024

Seems that scipy had a from numpy import * which was removed after scipy v1.3.3.

What I don't understand is what has changed to make this issue appear now.
Pointing to scipy 1.3.3 is likely to cause many other issues.

@FranzForstmayr
Copy link
Collaborator

I don't think it was removed, at least it was able to import linspace from 1.11.4 without error or warning.

Python 3.12.1 (tags/v3.12.1:2305ca5, Dec  7 2023, 22:03:25) [MSC v.1937 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import scipy
>>> scipy.__version__
'1.11.4'
>>> from scipy import linspace
>>>
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from scipy import linspace
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'linspace' from 'scipy' (/home/franz/.cache/pypoetry/virtualenvs/test-OHDMMxWo-py3.10/lib/python3.10/site-packages/scipy/__init__.py)

There was no deprecation warning in 1.11, that's why we missed to change that earlier.
@jhillairet
The main problem is that a fresh skrf venv fails to import skrf now. We should make a new release asap IMO.

@Vinc0110
Copy link
Collaborator Author

The main problem is that a fresh skrf venv fails to import skrf now. We should make a new release asap IMO.

Is it because of my fix to import linspace from numpy instead of scipy in taper.py? My question is: was it a problem I merged this change so quickly? Do we need to undo it?

@FranzForstmayr
Copy link
Collaborator

The main problem is that a fresh skrf venv fails to import skrf now. We should make a new release asap IMO.

Is it because of my fix to import linspace from numpy instead of scipy in taper.py? My question is: was it a problem I merged this change so quickly? Do we need to undo it?

Your fix is perfectly fine, but unreleased. So when installing via pip you'll get the old version which tries to import linspace from scipy.

@Vinc0110
Copy link
Collaborator Author

So then maybe a super quick version 0.30.1 release? We should probably take a bit more time to prepare v1.0.0. @jhillairet ?

@jhillairet
Copy link
Member

So then maybe a super quick version 0.30.1 release? We should probably take a bit more time to prepare v1.0.0. @jhillairet ?

Yeah, I think so. Will prepare the next release asap.

@pjg0707
Copy link

pjg0707 commented Jan 22, 2024

Setting the scipy version to 1.11.4 instead of 1.12 also worked for me.
Something must have been allowing the linspace to be accessible up to that version and it was changed in the latest release. Good that it was soon identified 👍

SMoraisAnsys added a commit to ansys/pyaedt that referenced this issue Jan 23, 2024
There is a side effect on scikit-rf 0.30.0 as it uses scipy without
bounding the version and that method linspace, used by scikit-rf
is no longuer available through scipy import in scipy 1.12.0.
(see scikit-rf/scikit-rf#1005)

Note: this should be 'reverted' later on to use scikit-rf new
version that should fix that issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants