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

MAINT: allow users to import id constants from api #508

Merged
merged 11 commits into from
Mar 10, 2023
3 changes: 3 additions & 0 deletions envisage/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@
from .i_plugin_manager import IPluginManager
from .i_service_registry import IServiceRegistry

from .ids import BINDINGS, COMMANDS, PREFERENCES, PREFERENCES_CATEGORIES, \
Copy link
Member

Choose a reason for hiding this comment

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

Style note: on this project, we tend to break up long import lines to place the imports one-per-line. For an example, see the from .extension_point_binding line below.

I'd also like to keep the imports alphabetically ordered as far as possible - this makes it easier for a code reader to scan the set of imports to see if something's there or not (and also helps to minimise merge conflicts in the event that two separate PRs both touch the import list). Please could you move this addition so that it's just before the from .import_manager import?

PREFERENCES_PANES, SERVICE_OFFERS, TASKS, TASK_EXTENSIONS

from .application import Application
from .core_plugin import CorePlugin
from .egg_plugin_manager import EggPluginManager
Expand Down
27 changes: 27 additions & 0 deletions envisage/tests/test_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# (C) Copyright 2007-2023 Enthought, Inc., Austin, TX
# All rights reserved.
#
# This software is provided without warranty under the terms of the BSD
# license included in LICENSE.txt and may be redistributed only under
# the conditions described in the aforementioned license. The license
# is also available online at http://www.enthought.com/licenses/BSD.txt
#
# Thanks for using Enthought open source!

import unittest
mdickinson marked this conversation as resolved.
Show resolved Hide resolved
import envisage.api as api
import envisage.ids as ids


class ApiTestCase(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

I suggest TestApi, to match the name of the module. (We do have a few existing *TestCase names, but in new code we should migrate away from that pattern.)

Suggested change
class ApiTestCase(unittest.TestCase):
class TestApi(unittest.TestCase):

""" Test for API """
def test_import(self):
self.assertEqual(api.BINDINGS, ids.BINDINGS)
self.assertEqual(api.COMMANDS, ids.COMMANDS)
self.assertEqual(api.PREFERENCES, ids.PREFERENCES)
self.assertEqual(api.PREFERENCES_CATEGORIES,
ids.PREFERENCES_CATEGORIES)
self.assertEqual(api.PREFERENCES_PANES, ids.PREFERENCES_PANES)
self.assertEqual(api.SERVICE_OFFERS, ids.SERVICE_OFFERS)
self.assertEqual(api.TASKS, ids.TASKS)
self.assertEqual(api.TASK_EXTENSIONS, ids.TASK_EXTENSIONS)