Skip to content

Add prototype namespace and datasets#4433

Closed
pmeier wants to merge 14 commits intomainfrom
prototype
Closed

Add prototype namespace and datasets#4433
pmeier wants to merge 14 commits intomainfrom
prototype

Conversation

@pmeier
Copy link
Contributor

@pmeier pmeier commented Sep 16, 2021

No description provided.

* add MNIST and CIFAR

* fix CI

* fix decoder test

* update after ZipArchiveReader changes
* refactor DatasetConfig

* add tests
* [PROTOTYPE] add more tests for utils and core API

* add pytest-mock as test requirement
* [PROTOTYPE] add more tests for utils and core API

* [PROTOTYPE] add caltech256 dataset
* [PROTOTYPE] add support for categories in DatasetInfo

* fix setup
@@ -0,0 +1,5 @@
from ._home import *
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought asterisk import is prohibited by flake8. Are there such many symbols to be imported here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought asterisk import is prohibited by flake8.

Yes, but we blanket ignore F403:

ignore = F401,E402,F403,W503,W504,F821

I'm in the process of fixing that, but I think that is ok in __init__.py files under the condition that the modules define an __all__. This way all the information about what is exposed is contained in the module itself.

Are there such many symbols to be imported here?

Nope, but this way the file stays clean. Following my advice above the namespace also stays clean. The only issue with the style used here is that by looking at the __init__.py we don't know where something is defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some more thought, I think you are right. We should be explicit there.

@pmeier
Copy link
Contributor Author

pmeier commented Sep 20, 2021

@mthrok We are going to split this PR up into more reviewable chunks. Do you want me to cc you there?

@mthrok
Copy link
Contributor

mthrok commented Sep 20, 2021

@mthrok We are going to split this PR up into more reviewable chunks. Do you want me to cc you there?

sure, thanks.

@pmeier pmeier closed this Oct 26, 2021
@pmeier pmeier deleted the prototype branch October 26, 2021 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants