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

`range` function cannot be overwritten using factory function #1162

Closed
weinshel opened this Issue Jul 10, 2018 · 7 comments

Comments

Projects
None yet
2 participants
@weinshel
Contributor

weinshel commented Jul 10, 2018

I am unable to override the range function using a factory function, even though I am able to override other functions using factory functions and override range using a non-factory function.
See this gist for a minimal example, which shows:

  • I am able to override other functions, such as reshape, using a factory function
  • I am able to override range using a non-factory function
  • If I override range using a non-factory function, then override it using a factory function, the definition from the factory function is used.

@josdejong josdejong added the bug label Jul 10, 2018

@josdejong

This comment has been minimized.

Show comment
Hide comment
@josdejong

josdejong Jul 10, 2018

Owner

Thanks for reporting and the clear code example!

This issue may have to do with range having a transform function defined.

Owner

josdejong commented Jul 10, 2018

Thanks for reporting and the clear code example!

This issue may have to do with range having a transform function defined.

@weinshel

This comment has been minimized.

Show comment
Hide comment
@weinshel

weinshel Jul 10, 2018

Contributor

That makes sense, it also fails for other functions with a transform such as mean. I may have narrowed in on a fix, I'll open a PR.

Contributor

weinshel commented Jul 10, 2018

That makes sense, it also fails for other functions with a transform such as mean. I may have narrowed in on a fix, I'll open a PR.

@josdejong

This comment has been minimized.

Show comment
Hide comment
@josdejong

josdejong Jul 10, 2018

Owner

working on it already...

Owner

josdejong commented Jul 10, 2018

working on it already...

@josdejong

This comment has been minimized.

Show comment
Hide comment
@josdejong

josdejong Jul 10, 2018

Owner

I have a fix in develop, see 6ac8807

Now order of loading of the transforms becomes important, which makes sense: first functions, then transforms of the functions (else the transforms are cleaned up again).

Owner

josdejong commented Jul 10, 2018

I have a fix in develop, see 6ac8807

Now order of loading of the transforms becomes important, which makes sense: first functions, then transforms of the functions (else the transforms are cleaned up again).

@weinshel

This comment has been minimized.

Show comment
Hide comment
@weinshel

weinshel Jul 10, 2018

Contributor

Ah perfect, I was getting to something similar, but good catch about the loading order. Thanks for the quick fix!

Contributor

weinshel commented Jul 10, 2018

Ah perfect, I was getting to something similar, but good catch about the loading order. Thanks for the quick fix!

@josdejong

This comment has been minimized.

Show comment
Hide comment
@josdejong

josdejong Jul 11, 2018

Owner

👍 thanks for your efforts too!

Owner

josdejong commented Jul 11, 2018

👍 thanks for your efforts too!

@josdejong josdejong closed this in 6ac8807 Jul 14, 2018

@josdejong

This comment has been minimized.

Show comment
Hide comment
@josdejong

josdejong Jul 14, 2018

Owner

Should be fixed now in v5.0.3

Owner

josdejong commented Jul 14, 2018

Should be fixed now in v5.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment