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

Remove unused dependencies #383

Merged
merged 6 commits into from
Oct 25, 2024

Conversation

bayc
Copy link
Collaborator

@bayc bayc commented Oct 23, 2024

Remove unused dependencies

This PR just removes currently unused dependencies. These were used for code in the alt_dev directory, developed by @qualand, who has approved removal of them for now and asked that we consider integrating them into future development. Also removed pyyaml-include as we use the load_yaml utility function now that uses the standard yaml package.

Related issue

Closes #303

Impacted areas of the software

pyproject.toml

Additional supporting information

None.

Test results, if applicable

Tests were passing on my fork. Looking into the failing tests now. Seems readthedocs is failing because of missing packages for the alt_dev directory. Confirming with @dguittet whether that directory is still needed or not.

@bayc bayc added dependencies medium priority Nice to have on the stove but not necessarily the front burner labels Oct 23, 2024
@bayc bayc added this to the NAWEA workshop-ready release milestone Oct 23, 2024
@bayc bayc self-assigned this Oct 23, 2024
@@ -15,29 +15,23 @@ dependencies = [
"NREL-PySAM==4.2.0",
"Pillow",
"Pyomo>=6.1.2",
"diskcache",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs build is failing because alt_dev requires this package. Was this just an unused import, or is it actually used in the model?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, just noticed that as well. It appears to be used in alt_dev, which I imagine several of the other packages were also used in. I've reached out to @dguittet to see if this directory should be kept around or not. Looks like the last development was 2 years ago (we moved some resource files 5 months ago, but that was done to the whole repo).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed with @qualand that the directory could be removed for the next release.

@bayc bayc marked this pull request as draft October 23, 2024 16:13
Copy link
Collaborator

@RHammond2 RHammond2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, though I'm a bit unfamiliar with FLORIS 4.0, so I'm trusting the implementation is corrected now.

@RHammond2
Copy link
Collaborator

This looks good to me, though I'm a bit unfamiliar with FLORIS 4.0, so I'm trusting the implementation is corrected now.

No idea how I commented on the wrong PR, please ignore this comment.

@johnjasa johnjasa marked this pull request as ready for review October 25, 2024 13:51
@bayc bayc added high priority Need to tackle soon and removed medium priority Nice to have on the stove but not necessarily the front burner labels Oct 25, 2024
@johnjasa johnjasa merged commit ce8e81f into NREL:develop Oct 25, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants