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.

Sign up for GitHub

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

Jump to bottom

Replace compose class with compose function in functional.py #12

Open
quipa opened this issue Jun 9, 2018 · 1 comment
Open

Replace compose class with compose function in functional.py #12

quipa opened this issue Jun 9, 2018 · 1 comment

Comments

@quipa
Copy link

quipa commented Jun 9, 2018

It seems compose class could be converted to a function and use the initialization arguments as named arguments for the function instead.

Also the class name does not follow the default PEP 8 class naming CapWords convention (although PEP 8 does accept classes that are mainly used as callables to use the function naming convention). For regular Python users this can be confusing.
@quipa quipa changed the title Replace compose class with compose function in functional.py Replace compose class with compose function in functional.py Jun 9, 2018
@ankostis
Copy link

ankostis commented Sep 29, 2019 •
edited
Loading

That architectural change would require some effort, instead of overriding __call__, to a function returning a function (i.e. if we were to preserve API).
The current situation [edit: is akin to the "builder pattern" which is a nice UAPI UX, and ] the implementation is not PEP-8 invalid, as you wrote.
So what would be the benefit of a functional, function factory?

ankostis added a commit to pygraphkit/graphtik that referenced this issue Nov 24, 2019
@ankostis
BREAK(NETOP): PRUNE permanently based on needs/provides ...,
fcc839c 
so as not to have to pass them on each call.

+ BREAK(netop): simplify `compose` API by turning it from class --> function;
  all args are now given in a single `compose()` call.
  (yahoo#12)
+ FIX: netop `needs` are not _optional_ by default; optional becomes
  only if all underlying operations have a certain need as optional.
+ feat(netop): +copy() method for setting pruning needs/provides.
ankostis added a commit to pygraphkit/graphtik that referenced this issue Nov 25, 2019
@ankostis
BREAK(NETOP): PRUNE permanently based on needs/provides ...,
f142216 
so as not to have to pass them on each call.

+ BREAK(netop): simplify `compose` API by turning it from class --> function;
  all args are now given in a single `compose()` call.
  (yahoo#12)
+ FIX: netop `needs` are not _optional_ by default; optional becomes
  only if all underlying operations have a certain need as optional.
+ feat(netop): +copy() method for setting pruning needs/provides.
ankostis added a commit to pygraphkit/graphtik that referenced this issue Nov 25, 2019
@ankostis
BREAK(NETOP): PRUNE permanently based on needs/provides ...,
6ed5709 
so as not to have to pass them on each call.

+ BREAK(netop): simplify `compose` API by turning it from class --> function;
  all args are now given in a single `compose()` call.
  (yahoo#12)
+ FIX: netop `needs` are not _optional_ by default; optional becomes
  only if all underlying operations have a certain need as optional.
+ feat(netop): +copy() method for setting pruning needs/provides.
ankostis added a commit to pygraphkit/graphtik that referenced this issue Nov 25, 2019
@ankostis
BREAK(NETOP): PRUNE permanently based on needs/provides ...,
d71d893 
so as not to have to pass them on each call.

+ BREAK(netop): simplify `compose` API by turning it from class --> function;
  all args are now given in a single `compose()` call.
  (yahoo#12)
+ FIX: netop `needs` are not _optional_ by default; optional becomes
  only if all underlying operations have a certain need as optional.
+ feat(netop): +copy() method for setting pruning needs/provides.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ankostis @quipa