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

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

Closed
markcampanelli opened this issue Jan 8, 2024 · 7 comments · Fixed by #37
Closed

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

markcampanelli opened this issue Jan 8, 2024 · 7 comments · Fixed by #37

Comments

@markcampanelli
Copy link
Contributor

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
Copy link
Collaborator

Hi Mark,

Absolutely, that would be a nice improvement.

@markcampanelli
Copy link
Contributor Author

@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
Copy link
Contributor Author

markcampanelli commented Jan 10, 2024

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).

@markcampanelli
Copy link
Contributor Author

markcampanelli commented Jan 10, 2024

@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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Contributor Author

@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.

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 a pull request may close this issue.

2 participants