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

Rename ghost'' to ghostWith #967

Merged
merged 1 commit into from Dec 20, 2022
Merged

Rename ghost'' to ghostWith #967

merged 1 commit into from Dec 20, 2022

Conversation

Zalastax
Copy link
Collaborator

@Zalastax Zalastax commented Nov 12, 2022

Keeps ghost'' as an alias.
Closes #958

@mindofmatthew
Copy link
Contributor

Looking through the issues, I think this actually closes #958?

@Zalastax
Copy link
Collaborator Author

Good catch! I'll update the commit to fix this typo.

Keeps ghost'' as an alias.
Closes tidalcycles#958
@mindofmatthew
Copy link
Contributor

I don't have approval permission, but I think this is worth merging. In general, I think v1.10 could benefit from cleaning up and standardizing the API a bit. Tidal currently exports 800 functions and 1300 params, only a fraction of which are documented. I'd propose the following guidelines for this process:

  • Retain all old names as aliases for now
  • Prefer camelCase version in function definitions and documentation (with lowercase aliases in code)
  • Move away from "prime" function notation (func' or func'') (as it doesn't communicate anything about the function itself and it can't be represented in Strudel)
  • Adopt a more consistent vocabulary, eg:
    • funcBy generally means that an operation has an extra numeric argument, specifying how much or by what probability an operation is applied
    • funcWith generally means that an operation has an extra functional argument, specifying a function that further modifies the added/generated/modified pattern
  • Maintain parity with Strudel
  • Identify which functions are internal and explicitly exclude them from export lists

Anything I'm missing? I can do a first pass and drop a list of specific suggestions in an issue for folks to comment on, unless there's some other way we should approach this.

@yaxu yaxu merged commit 08aad6c into tidalcycles:main Dec 20, 2022
@yaxu
Copy link
Member

yaxu commented Dec 20, 2022

Ok merged!

@mindofmatthew thanks that's well thought through, I agree with every one of those points!

The complication is that I've put a lot of work into #947 on the cycseq branch, which is a major reorganisation towards Tidal version 2.0, based on the Tidal 'remake' (that went into vortex and strudel). Would you be up for looking at that instead? I haven't looked at it in a little while, it's not complete but it's passing most tests. It could be a good basis for standardisation.

@mindofmatthew
Copy link
Contributor

@yaxu Yes, I can definitely focus my attention on the cycseq branch, though I'd be inclined to submit parallel PRs against main if any particularly useful changes come up. I agree that 2.0 is the place for a broad reorganization.

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.

rename ghost'' to ghostWith
3 participants