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

Addressing #420 (for CCI Toolbox) #421

Merged
merged 12 commits into from
Mar 17, 2021
Merged

Addressing #420 (for CCI Toolbox) #421

merged 12 commits into from
Mar 17, 2021

Conversation

forman
Copy link
Member

@forman forman commented Mar 10, 2021

  • Slightly changed signature of xcube.core.store.DataStore.get_dataset_ids()
    by adding a new keyword argument include_attrs: Sequence[str] = None that
    can be used to obtain a minimum set of dataset attributes for each returned
    dataset identifier. However, include_attrs is ignored to far in the "s3",
    "memory", and "directory" data stores. (DataStore.get_data_ids() to return minimum set of attributes #420)

Checklist:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/source/*
  • Changes documented in docs/CHANGES.md
  • AppVeyor and Travis CI passes
  • Test coverage remains or increases (target 100%)
  • Associated issues closed after merge

Copy link
Contributor

@TonioF TonioF left a comment

Choose a reason for hiding this comment

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

I like it, but please consider my comments.

omitted, all available data identifiers are returned.
:param include_titles: If true, the store will attempt to also provide a title.
If a store implementation supports only a single data type, it should verify that
*type_specifier* is either None or compatible with the supported data type.
Copy link
Contributor

Choose a reason for hiding this comment

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

If a store implementation supports only a single data type, it should verify that type_specifier is either None or compatible with the supported data type. -> A store implementation should verify that type_specifier is either None or compatible with a supported data type.

@TonioF
Copy link
Contributor

TonioF commented Mar 11, 2021

One more thing: Four tests fail.

@TonioF
Copy link
Contributor

TonioF commented Mar 11, 2021

And, I'm sorry: Where do we put the information what attributes a store offers?

TonioF and others added 4 commits March 12, 2021 13:28
Co-authored-by: Tonio Fincke <tonio.fincke@brockmann-consult.de>
Co-authored-by: Tonio Fincke <tonio.fincke@brockmann-consult.de>
@forman
Copy link
Member Author

forman commented Mar 16, 2021

And, I'm sorry: Where do we put the information what attributes a store offers?

We'll define a set of common and generic attributes that may be requested here. We put them in the method's docstring later.

@TonioF TonioF linked an issue Mar 17, 2021 that may be closed by this pull request
@TonioF TonioF merged commit 1348ea3 into master Mar 17, 2021
@TonioF TonioF deleted the forman-420-include_attrs branch March 17, 2021 10:17
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.

DataStore.get_data_ids() to return minimum set of attributes
2 participants