Skip to content

Conversation

@DavePutz
Copy link
Collaborator

@DavePutz DavePutz commented May 5, 2020

Issue #2812 - throw a NotImplementedError instead of "AttributeError: 'module' object has no attribute" for time functions that require long ints on platforms that do not support those.
It will now look like:

>>> import time
>>> dir(time)
['__class__', '__name__', 'localtime', 'mktime', 'monotonic', 'monotonic_ns', 'sleep', 'struct_time', 'time']
>>> time.time()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NotImplementedError: No long integer support

Issue #2812 - throw a NotImplementedError instead of "AttributeError: 'module' object has no attribute" for time functions that require long ints on platforms that do not support those.
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Sorry, I should have been clearer in #2812, I was thinking more like this:

STATIC mp_obj_t time_not_implemented(void) {
    mp_raise_NotImplementedError(translate("No long integer support"));
}
MP_DEFINE_CONST_FUN_OBJ_0(time_not_implemented_obj, time_not_implemented);

and then reference that in `time_modules_global_table, for each unimplemented function, e.g., for one of them:

    { MP_ROM_QSTR(MP_QSTR_time), MP_ROM_PTR(&time_not_implemented_obj) },

Locally, I tried using #if inside each function to select either the longint impl or an mp_raise_NotImplementedError(), but that seems to generate more code.

@dhalbert
Copy link
Collaborator

dhalbert commented May 5, 2020

By the way, I would suggest that you not work on the master branch in your fork. Instead, create a branch for this particular fix, and commit to that branch. Then your master won't get out of sync with upstream. See https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github/always-work-on-a-branch.

Also, you will need to do make translate before pushing your changes, because you've added a new translate() string. Otherwise the builds will fail.

@DavePutz
Copy link
Collaborator Author

Fixed in PR #2871

@DavePutz DavePutz closed this May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants