-
Notifications
You must be signed in to change notification settings - Fork 28
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 unlicensed dependency and unused code #105
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
This is unrelated to this PR, but: does it matter if we point to a dependency that doesn't have a license, if we're not distributing it ourselves? |
@@ -8,21 +8,17 @@ black = "*" | |||
|
|||
[packages] | |||
bokeh = "==2.0.2" | |||
h5py = "==2.10.0" | |||
h5pyd = "==0.3.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we no longer need h5py
, do we still need h5pyd
? We're also pointing to a version that's nearly two years out of date...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is used in: prereise/gather/solardata/ga_wind/ga_wind.py
. It is needed to retrieve data from a NREL data base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are still using h5pyd
in ga_wind.py
. Guess it is very stable :) Apparently it's also independent of h5py
(looks like it's a rest api client for reading data remotely).
I wonder the same thing, but not a lawyer so just going off what Ben/Kaspar have said, I think after talking to a real law talkin' guy. |
@kasparm talked to a lawyer. Maybe he can comment on this. |
Purpose
Delete te_wind folder since we aren't using it, and remove pywtk dependency which doesn't have a license. Checked for other unused packages and also removed ones where I found no usages.
What it does
Update package files, delete folder, remove te_wind from
__init__.py
and remove corresponding section from readme.Time to review
10 min - could use some double checking to make sure I didn't miss any usages in notebooks (I didn't fully check all of them, but did a text search on the project). Tests still pass but not sure how much coverage we have here.