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

Change Selector.by_leaves() to Selector.by_leaf() #698

Closed
trevorbaca opened this issue Jun 19, 2016 · 10 comments · Fixed by #714
Closed

Change Selector.by_leaves() to Selector.by_leaf() #698

trevorbaca opened this issue Jun 19, 2016 · 10 comments · Fixed by #714
Assignees

Comments

@trevorbaca
Copy link
Member

Selector methods are named in the singular:

Selector.by_class()
Selector.by_logical_measure()
Selector.by_logical_tie()
Selector.by_run()

These accord with IterationAgent method names:

IterationAgent.by_class()
IterationAgent.by_logical_tie()
IterationAgent.by_logical_voice()
IterationAgent.by_run()

Selector.by_leaves() appears to be a small oversight and might better be changed to Selector.by_leaf() to harmonize with IterationAgent.by_leaf() and the other existing methods.

@trevorbaca trevorbaca self-assigned this Jun 19, 2016
@josiah-wolf-oberholtzer
Copy link
Member

👍 Sounds good to me.

@trevorbaca
Copy link
Member Author

Ok cool. Wanted to make sure there wasn't a naming convention I was missing ;)

Will make the change in my current score (ie, cleanup) branch and will merge into master when the branch is outta the oven.

@josiah-wolf-oberholtzer
Copy link
Member

Cool. We should talk before we start merging stuff in, since my cleanup branch is getting pretty full too.

@trevorbaca
Copy link
Member Author

Ok. If you want me to merge sooner rather than later, lemme know!

@josiah-wolf-oberholtzer
Copy link
Member

No, no. Later is better. I want to teach you rebasing first.

I also wanted to do a little bit of polishing on some of the changes I saw in your cleanup branch, to make the spanner changes easier for users to adapt to. There's a PR for merging into your cleanup branch that extends attach() such that it iterates leaves prior to attaching spanners, so users don't need to change their code so much.

@trevorbaca
Copy link
Member Author

Cool. I saw that, which is great. Let's also make a (user-settable) flag, so that users can turn it on and off and -- hopefully -- eventually migrate entirely to the new way of attaching. (The convenience of the attach coercion is awesome! Just don't want users to slowly develop a false mental model of what's ultimately attaching to what.)

@josiah-wolf-oberholtzer
Copy link
Member

I wonder about that though. The more composing I do in the system, there
more I find it often overly strict in how it enforces signatures. There are
so many useful and really productive opportunities for coercion that we've
restricted I suppose really just for documentation purposes.

On Sun, Jun 19, 2016, 2:11 PM Trevor Bača notifications@github.com wrote:

Cool. I saw that, which is great. Let's also make a (user-settable) flag,
so that users can turn it on and off and -- hopefully -- eventually migrate
entirely to the new way of attaching. (The convenience of the attach
coercion is awesome! Just don't want users to slowly develop a false mental
model of what's ultimately attaching to what.)


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#698 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AFQCis2XxS27GJOfyFLr3Fb90qAo4LZTks5qNbBqgaJpZM4I5P-b
.

@josiah-wolf-oberholtzer
Copy link
Member

And if there's only one kind of component spanners can attach to ever, why make users do the work of always collecting the correct components? Does enforced demonstration of knowledge of exactly how the system works outweigh the practicality of getting work done efficiently?

@trevorbaca
Copy link
Member Author

I'm with you. The language hasn't ever really allowed for first-class signature polymorphism (as compared to, say, C++ or Mathematica). Which is probably what explains why coercion sometimes adds to our documentation burden: we add coercion features when what we really want is signature polymorphism. But signature polymorphism isn't recognized by Sphinx and friends (because it's never been a priority in Python itself). I've often wondered if the Zen-in-Python maxim of the principle-of-least-surprise (what PEP 20 states as "explicit is better than implicit") has been one of the language gods' drivers over the years that's kept more flexible signatures out of the language for so long.

At any rate, probably best to keep the thinking focused on concrete cases (like your extensions to attach() which is a great example). That way we can keep moving forward without getting stuck on much larger (albeit truly interesting) principles! ;)

@josiah-wolf-oberholtzer
Copy link
Member

With you 100%

And I'm sure we can find a way towards better polymorphic documentation of we dedicate ourselves to the problem actively.

@trevorbaca trevorbaca changed the title Change Selector.by_leaves() to Selector.leaf() Change Selector.by_leaves() to Selector.by_leaf() Jun 23, 2016
trevorbaca added a commit that referenced this issue Jun 23, 2016
…HNAGE.

This closes #698.

CHANGE MANAGEMENT. Change should be communicated clearly to user base.
josiah-wolf-oberholtzer pushed a commit that referenced this issue Jul 4, 2016
…HNAGE.

This closes #698.

CHANGE MANAGEMENT. Change should be communicated clearly to user base.
josiah-wolf-oberholtzer pushed a commit that referenced this issue Jul 4, 2016
…HNAGE.

This closes #698.

CHANGE MANAGEMENT. Change should be communicated clearly to user base.
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 a pull request may close this issue.

2 participants