Skip to content

ADD: Adding ameriflux discovery module. This module is based on the R code amerifluxr#916

Merged
zssherman merged 26 commits into
ARM-DOE:mainfrom
zssherman:ameriflux_add
May 21, 2025
Merged

ADD: Adding ameriflux discovery module. This module is based on the R code amerifluxr#916
zssherman merged 26 commits into
ARM-DOE:mainfrom
zssherman:ameriflux_add

Conversation

@zssherman

@zssherman zssherman commented Apr 2, 2025

Copy link
Copy Markdown
Collaborator
  • Tests added
  • Documentation reflects changes
  • PEP8 Standards or use of linter
  • Xarray Dataset or DataArray variable naming follows 'ds' or 'da' naming

@zssherman

Copy link
Copy Markdown
Collaborator Author

Note, I still need to add tests and examples

@zssherman

Copy link
Copy Markdown
Collaborator Author

@AdamTheisen Should we leave the default to agree_policy False? So users read the policy first? I was debating how to approach that...

@AdamTheisen

Copy link
Copy Markdown
Collaborator

@zssherman I didn't get a chance to review today but blocking my calendar Wed afternoon

@zssherman

Copy link
Copy Markdown
Collaborator Author

@AdamTheisen no worries!

Comment thread act/discovery/ameriflux.py Outdated
Comment thread act/discovery/ameriflux.py Outdated
Comment thread act/discovery/ameriflux.py
@zssherman

zssherman commented Apr 9, 2025

Copy link
Copy Markdown
Collaborator Author

@AdamTheisen I did changes to address your comments. Let me know if the recent commit looks good for all your comments. For the is test I add a kwargs, but once we have the seceret email and username added, ill add one more check to the is test. That way we keep that hidden, but still available for our unit tests

@zssherman

Copy link
Copy Markdown
Collaborator Author

@AdamTheisen Added the new reader, ready for review when folks have time. Working on the tests still though

@AdamTheisen AdamTheisen left a comment

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.

@zssherman I cc'd you in on the email from the AmeriFlux developers and I think you hit all their requirements. I think this is looking good as well once we get the tests working!

@zssherman zssherman closed this May 5, 2025
@zssherman zssherman reopened this May 5, 2025
@zssherman zssherman closed this May 13, 2025
@zssherman zssherman reopened this May 13, 2025
@zssherman zssherman merged commit cd8bd8a into ARM-DOE:main May 21, 2025
30 of 40 checks passed
@zssherman zssherman deleted the ameriflux_add branch May 28, 2025 23:38
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.

2 participants