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

start work on results classes #860

Closed
wants to merge 1 commit into from
Closed

Conversation

ebolch
Copy link

@ebolch ebolch commented Oct 31, 2024

This is what @jhkennedy, @JessicaS11 and I started work on during the 2024-10-29 Hackday.

The end goal is to return a results class object rather than a list, using the existing DataCollections / DataGranules classes. This could enable results to be combined (and possibly filtered to only certain assets?)

The first step we discussed/started was adding a load method to store actual results and a granules property to allow the object to still function as a list.

The long-term plan would have functions like download and open act on this object rather than expecting the user to supply granules and provider inputs.

There is more discussion here.

Pull Request (PR) draft checklist - click to expand
  • Please review our
    contributing documentation
    before getting started.
  • Populate a descriptive title. For example, instead of "Updated README.md", use a
    title such as "Add testing details to the contributor section of the README".
    Example PRs: #763
  • Populate the body of the pull request with:
  • Update CHANGELOG.md with details about your change in a section titled
    ## Unreleased. If such a section does not exist, please create one. Follow
    Common Changelog for your additions.
    Example PRs: #763
  • Update the documentation and/or the README.md with details of changes to the
    earthaccess interface, if any. Consider new environment variables, function names,
    decorators, etc.

Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!

Pull Request (PR) merge checklist - click to expand

Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the @nsidc/earthaccess-support team in a comment and we
will help you out!

  • Add unit tests for any new features.
  • Apply formatting and linting autofixes. You can add a GitHub comment in this Pull
    Request containing "pre-commit.ci autofix" to automate this.
  • Ensure all automated PR checks (seen at the bottom of the "conversation" tab) pass.
  • Get at least one approving review.

📚 Documentation preview 📚: https://earthaccess--860.org.readthedocs.build/en/860/

Copy link

Binder 👈 Launch a binder notebook on this branch for commit d4505ed

I will automatically update this comment whenever this PR is modified

@jhkennedy
Copy link
Collaborator

@ebolch great! Thanks for opening the PR!

How would you live to move forward with it? I can spend a little time hacking and leave some TODOs to circle back to, just outline a gameplan, or we can wait until the next hackday to team up on it? I'm happy to drive or let you drive.

@ebolch
Copy link
Author

ebolch commented Oct 31, 2024

@jhkennedy I'm pretty flexible on how we move forward. I'm planning to spend some time working on this next week, but some guidance on how to make the changes would be great. Feel free to do some hacking or outline TODOs if you have time, otherwise I'll try to make some progress and then we can continue next hackday.

@JessicaS11
Copy link
Collaborator

@ebolch @jhkennedy Likewise I'd be happy to spend some time contributing to this (and could do so this week), so if it's helpful to assign me a few to-dos, please do!

@jhkennedy
Copy link
Collaborator

I'm going to close this PR, push this branch directly to nsidc/earthaccess, and open a new PR so that we can collaborate on this feature more easily.

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.

3 participants