Closed hagenw closed 4 months ago
- When creating from a new
venv
, I find that the requirementdohq-artifactory
is missing and needs to be added using apip install
. For me, it would be ok to add this change here, but as well it would be ok to create a follow-up issue that should be processed pre-release.
This should not happen. audb
is a dependency of audbcards
, and has dohq-artifactory
as dependency, see https://github.com/audeering/audb/blob/916c4e9c538b6b8f376835a2e59be46213d1f4aa/pyproject.toml#L36 and https://github.com/audeering/audbackend/blob/84b723c70bd92ddb4f05a97dc974397d07131d1c/pyproject.toml#L39
- There is an asymmetry in how the API docs of
Dataset
andDatacard
show up. While forDatacard
, properties are rendered, this is not the case forDataset
as its only purpose is to delegate object instantiation to its private counterpart_Dataset
. In a follow up one should probably exposeDataset
properties and make these accessible to the documentation as this is likely to be of interest to the user.
Good point. As this is more complicated, I created https://github.com/audeering/audbcards/issues/66 to track it.
- docstring
file_duration_distribution
: would Min/max not require a period here to flag it as an abbrev.? I.e. Min -> Min. ?
I changed the docstring to
- The
Datacard.player
docstring for thefile
parameter: I am not sure whether I understand.
The player needs a file name as input. This is usually hidden from the user when a datacard is used as we automatically use audbcards.Datacard.example_media
. That's t he reason why I added the audbcards.Datacard.example_media is a good fit
part to the docstring for the file
argument. Should I remove the line or rephrase it?
The player needs a file name as input. This is usually hidden from the user when a datacard is used as we automatically use audbcards.Datacard.example_media. That's t he reason why I added the audbcards.Datacard.example_media is a good fit part to the docstring for the file argument. Should I remove the line or rephrase it?
Now I see. You want to say that the value of audbcards.Datacard.example_media
is very likely / sure to work if you want to parametrize it. I am not saying that I know the best way to phase it, but if you have a good way to do so it would be great of course. One could also make it default file to None
and use audbcards.Datacard.example_media
if it is None - so the type would become a union of None
and str
.
This should not happen. audb is a dependency of audbcards, and has dohq-artifactory as dependency, see https://github.com/audeering/audb/blob/916c4e9c538b6b8f376835a2e59be46213d1f4aa/pyproject.toml#L36 and https://github.com/audeering/audbackend/blob/84b723c70bd92ddb4f05a97dc974397d07131d1c/pyproject.toml#L39
I have used a python3.9 venv (3.10 working on a compute machine with only 18.04 installed has become less viable since deadsnakes has removed all support for 18.04). Possibly dependencies to not go down well with 3.9 and audb any more. So you can see this as resolved.
So if you want to rephrase the good fit
bit this is good to go (and merge it), if not you can also go ahead and merge.
Good idea, I set the default value of file
to None
in audbcards.Datacard.player()
.
I will now continue here and merge, thanks for the review.
When I start from an empty environment, created with virtualenv
and Python 3.9 and run pip install -r requirements.txt
in the root of this repo, it installs the following packages, including dohq-artifactory
:
This adds API documentation for
audbcards.Dataset
andaudbcards.Datacard
to the documentation. It sounds like a good idea to me, to add this before we do our first release.