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

Orange.data: Clean up the namespace #1813

Merged
merged 1 commit into from Dec 14, 2016

Conversation

Projects
None yet
4 participants
@lanzagar
Copy link
Contributor

commented Dec 9, 2016

Issue

Orange.data namespace included a lot of junk.

Description of changes

Define __all__ in data.io.

Includes
  • Code changes
  • Tests
  • Documentation

@lanzagar lanzagar force-pushed the lanzagar:fixdatanamespace branch from a1d97b8 to d692ae6 Dec 9, 2016

@@ -29,6 +29,9 @@
from Orange.util import Registry, flatten, namegen


__all__ = ["Flags"]

This comment has been minimized.

Copy link
@kernc

kernc Dec 9, 2016

Member

I don't think that's ok. The module defines a lot more "public" stuff than just Flags. Perhaps rather make import * in Orange.data.__init__.py explicit?

This comment has been minimized.

Copy link
@astaric

astaric Dec 10, 2016

Member

FileFormat might be considered public as well, as someone might use it to define their own readers, but what else? Reader implementations are not supposed to be used directly, they are accessible through FileReader.get_reader.

This comment has been minimized.

Copy link
@lanzagar

lanzagar Dec 12, 2016

Author Contributor

Added FileFormat. As said, the individual readers are really just current implementations that should not be used directly elsewhere, so I don't think they belong in __all__. And they can be imported from data.io if needed (for exceptional cases only).

This comment has been minimized.

Copy link
@kernc

kernc Jan 6, 2017

Member

but what else?

@astaric always more than one thinks.

https://github.com/biolab/orange3-text/pull/176/files

@lanzagar lanzagar force-pushed the lanzagar:fixdatanamespace branch from d692ae6 to c7d5d20 Dec 12, 2016

@codecov-io

This comment has been minimized.

Copy link

commented Dec 12, 2016

Current coverage is 89.05% (diff: 100%)

Merging #1813 into master will increase coverage by <.01%

@@             master      #1813   diff @@
==========================================
  Files            85         85          
  Lines          9029       9030     +1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           8041       8042     +1   
  Misses          988        988          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 002981b...05c1ef0

@@ -11,7 +11,7 @@
)
from AnyQt.QtGui import QIcon, QKeySequence, QDesktopServices

from Orange.data import FileFormat

This comment has been minimized.

Copy link
@astaric

astaric Dec 12, 2016

Member

This is not really needed anymore.

Orange.data: Clean up the namespace
Define __all__ in data.io.

@lanzagar lanzagar force-pushed the lanzagar:fixdatanamespace branch from c7d5d20 to 05c1ef0 Dec 13, 2016

@astaric astaric merged commit 7a745fe into biolab:master Dec 14, 2016

5 checks passed

codecov/patch 100% of diff hit (target 95.00%)
Details
codecov/project 89.05% (+<.01%) compared to 002981b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.