dandi / dandi-cli

DANDI command line client to facilitate common operations
https://dandi.readthedocs.io/
Apache License 2.0
22 stars 27 forks source link

get_dandisets: Expose options (embargoed, empty, draft) of the /dandisets/ endpoint #1413

Closed yarikoptic closed 8 months ago

yarikoptic commented 9 months ago

I thought that as long as I authenticate I would also get embargoed datasets listed, as I believe is done in web UI. But I found no Embargoed dandisets in returned list. Looking at our get_dandisets - apparently we do not expose any of the query parameters image for image

Immediate need is to get embargoed dandisets. I generally would not mind even adding "search" parameter.

jwodder commented 9 months ago

@jjnesbitt @mvandenburgh The "draft" and "user" query parameters to /dandisets/ are underdocumented in Swagger, and I need to document them in the Python API. What exactly do they do? (It appears that draft controls whether Dandisets without published versions are included, but I'd like to get confirmation.) Also, what exactly does search search?

@yarikoptic Should the names of the arguments to the get_dandisets() method match the query parameters? In particular, this endpoint's use of ordering is inconsistent with the name order used when fetching versions & assets (which I've previously complained about), so we might want to align all the arguments to order in the Python API.

yarikoptic commented 8 months ago

I am ok with your judgement on this matter: it seems we would be inconsistent either way. Ideally here or in https://github.com/dandi/dandi-archive/issues/1228#issuecomment-1204338808 we arrive at the target -- e.g. I propose: let's make it order and aim for it

order vs ordering is pretty much "equal match" within dandi-archive but in dandi-cli we have only (and many of) `order` ```shell ❯ git grep -p '\ *[:=]' dandiapi/api/views/asset.py=class AssetFilter(filters.FilterSet): dandiapi/api/views/asset.py: order = filters.OrderingFilter(fields=['created', 'modified', 'path']) dandiapi/api/views/version.py=class VersionFilter(filters.FilterSet): dandiapi/api/views/version.py: order = filters.OrderingFilter(fields=['created']) web/src/rest.ts=const dandiRest = { web/src/rest.ts: const versions = await this.versions(identifier, { page_size: 1, order: '-created' }); ❯ git grep -p '\ *[:=]' dandiapi/api/admin.py=class UserAdmin(BaseUserAdmin): dandiapi/api/admin.py: @admin.display(ordering='metadata__status') dandiapi/api/models/dandiset.py=class Dandiset(TimeStampedModel): dandiapi/api/models/dandiset.py: ordering = ['id'] dandiapi/api/views/dandiset.py=class DandisetFilterBackend(filters.OrderingFilter): dandiapi/api/views/dandiset.py: ordering = orderings[0] web/src/components/DandisetsPage.vue=export default defineComponent({ web/src/components/DandisetsPage.vue: const ordering = ((sortDir.value === -1) ? '-' : '') + sortField.value; web/src/views/DandisetLandingView/DandisetLandingView.vue=async function fetchNextPage() { web/src/views/DandisetLandingView/DandisetLandingView.vue: const ordering = ((sortDir === -1) ? '-' : '') + sortField; ❯ cd ../dandi-cli-master CHANGELOG.md LICENSE README.md build/ coverage.xml dandi.egg-info/ docs/ pyproject.toml setup.py* tox.ini versioneer.py DEVELOPMENT.md MANIFEST.in __pycache__/ codeql.yml dandi/ dist/ lgtm.yml setup.cfg tools/ venvs/ ❯ git grep -p '\ *[:=]' ❯ git grep -p '\ *[:=]' dandi/dandiapi.py=class RemoteDandiset: dandi/dandiapi.py: def get_versions(self, order: str | None = None) -> Iterator[Version]: dandi/dandiapi.py: for v in self.get_versions(order="-created"): dandi/dandiapi.py: def get_assets(self, order: str | None = None) -> Iterator[RemoteAsset]: dandi/dandiapi.py: self, path: str, order: str | None = None dandi/dandiapi.py: self, pattern: str, order: str | None = None dandi/dandiarchive.py=class ParsedDandiURL(ABC): dandi/dandiarchive.py: self, client: DandiAPIClient, order: str | None = None, strict: bool = False dandi/dandiarchive.py=class DandisetURL(ParsedDandiURL): dandi/dandiarchive.py: self, client: DandiAPIClient, order: str | None = None, strict: bool = False ... many more ... ```

Ideally in dandi-archive we deprecate ordering in API should be doable to add order to replace ordering but keep ordering available for backward compatibility to be pruned later.

WDYT @dandi/archive-maintainers ?

mvandenburgh commented 8 months ago

Thanks for pointing this out @jwodder, I made a PR on dandi-archive to improve the Swagger descriptions for this endpoint - #1875.

@jjnesbitt @mvandenburgh The "draft" and "user" query parameters to /dandisets/ are underdocumented in Swagger, and I need to document them in the Python API. What exactly do they do? (It appears that draft controls whether Dandisets without published versions are included, but I'd like to get confirmation.) Also, what exactly does search search?

You are correct about draft. The user param is kind of weird, in that it only has one valid value, "me". When set to "me", the endpoint will only return dandisets that the logged in user is an owner of. We should maybe reconsider how that's implemented on the API side. search does a string search over the json metadata of every Version in the system.

order vs ordering is pretty much "equal match" within dandi-archive but in dandi-cli we have only (and many of) order Ideally in dandi-archive we deprecate ordering in API should be doable to add order to replace ordering but keep ordering available for backward compatibility to be pruned later.

WDYT @dandi/archive-maintainers ?

I agree we should make these consistent.

github-actions[bot] commented 8 months ago

:rocket: Issue was released in 0.61.0 :rocket: