Skip to content

FARMS-DNI#11

Merged
bnb32 merged 55 commits intomainfrom
FARMS-DNI
Apr 13, 2023
Merged

FARMS-DNI#11
bnb32 merged 55 commits intomainfrom
FARMS-DNI

Conversation

@xieyupku
Copy link
Copy Markdown
Collaborator

The new DNI computation by FARMS-DNI

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 28, 2023

Codecov Report

Merging #11 (0d9baf6) into main (d252371) will increase coverage by 36.37%.
The diff coverage is 97.79%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main      #11       +/-   ##
===========================================
+ Coverage   20.94%   57.32%   +36.37%     
===========================================
  Files           7        9        +2     
  Lines         253      478      +225     
===========================================
+ Hits           53      274      +221     
- Misses        200      204        +4     
Flag Coverage Δ
unittests 57.32% <97.79%> (+36.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
farms/farms.py 0.00% <0.00%> (ø)
tests/test_farmsdni.py 96.00% <96.00%> (ø)
farms/farms_dni.py 100.00% <100.00%> (ø)
farms/version.py 100.00% <100.00%> (ø)

@bnb32 bnb32 requested review from bnb32 and grantbuster March 28, 2023 22:25
@bnb32
Copy link
Copy Markdown
Collaborator

bnb32 commented Mar 29, 2023

@grantbuster Yu and I have gone back and forth a bunch over doc strings and linting so I'll let you do the first review.

@bnb32
Copy link
Copy Markdown
Collaborator

bnb32 commented Mar 30, 2023

@xieyupku I don't think we need the farms/old directory, right? github will keep track of the history if that is why you kept it. I don't see that code being called anywhere.

farms/farms.py Outdated
return np.where(clear_mask, np.nan, ghi),\
np.where(clear_mask, np.nan, dni_farmsdni),\
np.where(clear_mask, np.nan, dni0)
# print( theta00, idx, muomega )
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this commented out code should be removed

@grantbuster
Copy link
Copy Markdown
Member

i dont think i commented this previously: we should increment the version before we merge this so we can do a release.

@bnb32 bnb32 force-pushed the FARMS-DNI branch 2 times, most recently from a87f0e1 to f5eb1e4 Compare April 13, 2023 22:46
@bnb32 bnb32 merged commit c154171 into main Apr 13, 2023
@bnb32 bnb32 deleted the FARMS-DNI branch April 13, 2023 22:58
github-actions bot pushed a commit that referenced this pull request Apr 13, 2023
@bnb32 bnb32 restored the FARMS-DNI branch April 13, 2023 23:12
@bnb32 bnb32 deleted the FARMS-DNI branch March 3, 2026 13:37
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 this pull request may close these issues.

4 participants