NREL / PVDegradationTools

Set of tools to calculate degradation responses and degradation related parameters for PV.
https://pvdegradationtools.readthedocs.io/
Other
24 stars 5 forks source link

pyproject.toml for builds, using setuptools_scm for versioning #36

Closed markcampanelli closed 7 months ago

markcampanelli commented 7 months ago

Hello PVdeg team and thanks for this effort!

I might be able to suggest a significantly more streamlined way of configuring, building, and testing this repo using a pyproject.toml file. I just want to check if the team is open to this if I can show that it would work.

martin-springer commented 7 months ago

Hi Mark,

Absolutely, that would be a nice improvement.

markcampanelli commented 7 months ago

@martin-springer I am working on this on a fork. I am getting a testing error related to KeyError: 'Dew Point' on the line psat, avg_psat = humidity.psat(PSM["Dew Point"]) (in test_edge_seal_ingress_rate and test_edge_seal_width). However, I DO see a column with that key in the .csv file. Seems unlikely my refactor broke this, but would you mind trying to reproduce this failure on your setup from origin/main?

https://github.com/markcampanelli/PVDegradationTools/actions/runs/7463413911/job/20308036151

markcampanelli commented 7 months ago

I was able to reproduce on the main branch of the ~origin~ upstream repo. Looks like the cause is this bugfix in pvlib that fixed the mapping of a "Dew Point" key to "dew_point": https://github.com/pvlib/pvlib-python/pull/1920. Using the dew_point key fixes the tests, but apparently only for pvlib >= v0.10.3 (https://github.com/pvlib/pvlib-python/blob/main/docs/sphinx/source/whatsnew/v0.10.3.rst).

markcampanelli commented 7 months ago

@martin-springer I have one issue to sort out before I move this PR out of draft for review. There is apparently not (yet?) a direct way to install a package into a conda environment using its pyproject.toml. (Instead of, say, a requirements.txt, see this.)

Thus, I'm not sure how this passage in the README should be updated:

  1. Create the environment and install the requirements. The repository includes a requirements.txt file that contains a list the packages needed to run this tutorial. To install them using conda run:
    conda create -n pvdeg jupyter -c pvlib --file requirements.txt
    conda activate pvdeg

    or you can install it with pip install pvdeg as explained in the installation instructions into the environment.

I'm inclined to just have folks install pvdeg using pip into the fresh conda environment. Last I knew, this works almost always.(IIRC, it's when doing a lot of mixing and matching between conda and pip that things can get squirrelly.)

martin-springer commented 7 months ago

I was able to reproduce on the main branch of the ~origin~ upstream repo. Looks like the cause is this bugfix in pvlib that fixed the mapping of a "Dew Point" key to "dew_point": pvlib/pvlib-python#1920. Using the dew_point key fixes the tests, but apparently only for pvlib >= v0.10.3 (https://github.com/pvlib/pvlib-python/blob/main/docs/sphinx/source/whatsnew/v0.10.3.rst).

Thank you, we'll have to update the version in the requirements.

martin-springer commented 7 months ago

@martin-springer I have one issue to sort out before I move this PR out of draft for review. There is apparently not (yet?) a direct way to install a package into a conda environment using its pyproject.toml. (Instead of, say, a requirements.txt, see this.)

Thus, I'm not sure how this passage in the README should be updated:

  1. Create the environment and install the requirements. The repository includes a requirements.txt file that contains a list the packages needed to run this tutorial. To install them using conda run:
conda create -n pvdeg jupyter -c pvlib --file requirements.txt
conda activate pvdeg

or you can install it with pip install pvdeg as explained in the installation instructions into the environment.

I'm inclined to just have folks install pvdeg using pip into the fresh conda environment. Last I knew, this works almost always.(IIRC, it's when doing a lot of mixing and matching between conda and pip that things can get squirrelly.)

Yes, that's good with me. We can just focus on using pip for now. We might add support for conda later on.

markcampanelli commented 7 months ago

@martin-springer Thanks. I will update the READMEs and test installation and running of the notebooks one last time in a fresh venv. (I don't have conda set up.)

Also, for the broken tests, I decided to add a dew-point key "fallback" to try to not force an immediate upgrade pvlib.