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

Installing 0.9.0 wheel on python 3.6 does not install dataclasses, causes import failure #1302

Closed
Tracked by #1424
kandersolar opened this issue Sep 2, 2021 · 7 comments · Fixed by #1422
Closed
Tracked by #1424

Comments

@kandersolar
Copy link
Member

kandersolar commented Sep 2, 2021

Describe the bug
Installing pvlib 0.9.0 using the wheel currently on PyPI does not install dataclasses, causing pvlib to fail to import on python 3.6. I think this is because the wheel builder workflow uses python 3.8, so it ignores the dataclasses dependency when building the wheel (which is supposed to be universal across all our supported python versions):

python-version: 3.8

pvlib-python/setup.py

Lines 49 to 50 in 518cc35

if sys.version_info.major == 3 and sys.version_info.minor == 6:
INSTALL_REQUIRES.append('dataclasses')

Opening the wheel (it's just a zipfile) and viewing the pvlib-0.9.0.dist-info/METADATA file, there is no mention of dataclasses. Note that installing 0.9.0 from source (pip install pvlib --no-binary pvlib) works as intended because it is running setup.py locally in python 3.6. Our CI never has this kind of python version mismatch between what was used to build the wheel and what was used to run the tests, so it didn't catch this.

To Reproduce

$ conda create -n py36test python=3.6
$ conda activate py36test
$ pip install pvlib==0.9.0
$ python -c "import pvlib"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\Users\KANDERSO\Software\Anaconda3\envs\py36test\lib\site-packages\pvlib\__init__.py", line 3, in <module>
    from pvlib import (  # noqa: F401
  File "C:\Users\KANDERSO\Software\Anaconda3\envs\py36test\lib\site-packages\pvlib\ivtools\__init__.py", line 7, in <module>
    from pvlib.ivtools import sde, sdm, utils  # noqa: F401
  File "C:\Users\KANDERSO\Software\Anaconda3\envs\py36test\lib\site-packages\pvlib\ivtools\sdm.py", line 16, in <module>
    from pvlib.pvsystem import calcparams_pvsyst, singlediode, v_from_i
  File "C:\Users\KANDERSO\Software\Anaconda3\envs\py36test\lib\site-packages\pvlib\pvsystem.py", line 14, in <module>
    from dataclasses import dataclass
ModuleNotFoundError: No module named 'dataclasses'

Additional context
Noticed in NREL/rdtools#290

Possible solutions:

  • Switch the deploy workflow to use python 3.6, which will end up requiring dataclasses for all python versions
  • Upgrade dataclasses to be required for all versions in setup.py
  • Build separate wheels for each python version, maybe with cibuildwheel?
  • Remove the wheel file so that pip always installs from source?
@wholmgren
Copy link
Member

I manually built the wheel using python 3.6 and uploaded to testpypi. It seems to work.

$ python setup.py sdist bdist_wheel --python-tag py36
$ twine upload --repository testpypi dist/pvlib-0.9.0-py36-none-any.whl
# new session
$ conda create -n pvlib36test python=3.6
$ conda activate pvlib36test
$ pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ pvlib
$ python
>>> import pvlib
>>> pvlib.__version__
'0.9.0'

As expected, running pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ pvlib in a python 3.8 environment gives me the pvlib 0.9 from primary pypi.

Should I go ahead and upload the py36 whl to primary pypi too or do you want to try cibuildwheel?

@kandersolar
Copy link
Member Author

Fine with me to upload the 36 wheel just to get this fixed in the short term. Will it conflict with the pvlib-0.9.0-py3-none-any.whl already there? I don't know how the wheel preference logic works when more than one wheel would work.

Long run I'll volunteer to set up cibuildwheel if we think that's the way to go.

@wholmgren
Copy link
Member

Uploaded. See here for documentation on order of preference.

@wholmgren
Copy link
Member

wholmgren commented Sep 2, 2021

I believe a simple solution is to remove

pvlib-python/setup.py

Lines 48 to 50 in 518cc35

# include dataclasses as a dependency only on python 3.6
if sys.version_info.major == 3 and sys.version_info.minor == 6:
INSTALL_REQUIRES.append('dataclasses')

and add the following to INSTALL_REQUIRES: 'dataclasses; python_version < "3.7"'.

I did that, rebuilt the wheel, and inspected the MANIFEST. It contained Requires-Dist: dataclasses ; python_version < "3.7". pip install dist/pvlib-0.9.0+0.g518cc355.dirty-py3-none-any.whl correctly installed dataclasses in a python 3.6 environment. The same command in a python 3.8 environment did not attempt to install dataclasses.

In any case, the conda-forge package is also broken on 3.6. I think that can also be solved using a similar syntax in the recipe/meta.yml file.

@kandersolar
Copy link
Member Author

Nice. Would it make sense to "re-release" 0.9.0 to PyPI using that syntax, or are you happy with the state of PyPI as-is? NREL/rdtools#290 is happy with it, so maybe no need to change anything.

I assume the pvlib conda channel has the same issue, so I'll take a look at that and conda-forge now.

@wholmgren
Copy link
Member

I'm happy with the current state so I'd say leave it alone.

@kandersolar
Copy link
Member Author

Ok, the pvlib and conda-forge channels have new builds (build 1) that work for me on both 3.6 and 3.7, so I think everything is fixed. If anyone discovers that's not the case, please let me know.

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.

2 participants