Repository navigation
Conversation
Hey @mokazemi! 🎉Thanks for opening your first pull request! We appreciate your If AI is used for any portion of this PR, you must vet the content |
c1c521f to
d6070ff
Compare
echedey-ls
left a comment
echedey-ls
left a comment
There was a problem hiding this comment.
There are other changes that must be done to to cec.py:79
- Update optional dependency from 'nrel-pysam' to 'nlr-pysam' - Uncomment nlr-pysam in Python 3.14 CI requirements - PySAM 8.0.0+ is distributed as nlr-pysam for Python 3.8-3.14 Note: test_fit_cec_sam shows minor numerical differences between nlr-pysam 8.0.0+ and nrel-pysam, particularly in the 'Adjust' parameter. This may indicate algorithmic changes in the newer nlr-pysam distribution that needs further investigation. Fixes pvlib#2871
d6070ff to
3c0187a
Compare
|
@mokazemi I believe @echedey-ls's comment still needs to be addressed: #2872 (review) |
Done. |
echedey-ls
left a comment
echedey-ls
left a comment
There was a problem hiding this comment.
@mokazemi I'm interested in knowing which AI toolchain you use. Are you on claude, codex, is the model any gpt, gemma, etc.? And are you using it for all commits or you only used it to kickstart the PR?
We will defer merging this PR until our CI is green once again. Meanwhile, feel free to list yourself in the relevant whatsnew file.
change package names to lowercase versions Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
I used GPT to diagnose the issue, Since I initially had issue when I was working on my project actually, when the system's Python upgraded and my venv couldn't resolve deps. |
* main: Note deprecation of king in get_total_irradiance and get_sky_diffuse docstrings (pvlib#2904) scaling.wvm: return unchanged signal for a single site (pvlib#2896) Fix the Series name of ModelChain DC ohmic losses (pvlib#2912) Fix test case naming for issue pvlib#2077 (pvlib#2909) Clarify multiple IV curve requirement for Sandia fitting methods (pvlib#2883) Use conda-forge for all conda CI tests (pvlib#2875) fix spelling typo in NumFocus alt-text (pvlib#2888) Fix `solar_position()` return statement docstring (pvlib#2887) Add dataset option to get_era5 (pvlib#2780) calc_surface_orientiation returns scalar when input is scalar (pvlib#2882)
|
@mokazemi can you please update this branch from pvlib/main? That should get the CI working again. Thanks! |
Changes
nrel-pysamtonlr-pysamnlr-pysamfor Python 3.8-3.14Testing
The existing test
test_fit_cec_samshows minor numerical differences when usingnlr-pysam8.0.0+:This may indicate algorithmic improvements or changes in nlr-pysam and should be reviewed by maintainers familiar with the PySAM library.
Closes #2871
nlr-pysampackage name #2871docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.