Hello
This is a little above my head, but it seems that Scale.all is now empty. When I look at the class implementation, it is supposed to populate, but it doesn’t. Scale.directory has now extra spaces before the ‘keys’… Am I just drunk? I can download an old version of SC, but I don’t understand how to troubleshoot basic code that is in the initClass and that seems to disapear and used to behave. Any pointer welcome
Can you check the stuff in initclass actually runs with a print statement? If it doesn’t run, there might be something up with extension methods.
Also, what version are you on?
The “basic code” for the Scale class hasn’t been changed since 2012. Seems this is extension/quark-related …
I did make a pr some time ago to remove that lil’ space in the .directory output. What “all” is supposed to do, I still don’t know…
On a fresh SC3.15.0-dev without any Quarks components:
Scale.all
returns
-> IdentityDictionary[]
Scale.directory
returns
-> \ aeolian: Aeolian
\ ahirbhairav: Ahirbhairav
\ ajam: Ajam
\ atharKurd: Athar Kurd
\ augmented: Augmented
\ augmented2: Augmented 2
\ bartok: Bartok
\ bastanikar: Bastanikar
\ bayati: Bayati
\ bhairav: Bhairav
\ chinese: Chinese
\ chromatic: Chromatic
\ chromatic24: Chromatic 24
\ diminished: Diminished
\ diminished2: Diminished 2
\ dorian: Dorian
\ egyptian: Egyptian
\ enigmatic: Enigmatic
\ farahfaza: Farahfaza
\ gong: Gong
\ harmonicMajor: Harmonic Major
\ harmonicMinor: Harmonic Minor
\ hexAeolian: Hex Aeolian
\ hexDorian: Hex Dorian
\ hexMajor6: Hex Major 6
\ hexMajor7: Hex Major 7
\ hexPhrygian: Hex Phrygian
\ hexSus: Hex Sus
\ hijaz: Hijaz
\ hijazDesc: Hijaz Descending
\ hijazKar: hijazKar
\ hindu: Hindu
\ hirajoshi: Hirajoshi
\ hungarianMinor: Hungarian Minor
\ husseini: Husseini
\ huzam: Huzam
\ indian: Indian
\ ionian: Ionian
\ iraq: Iraq
\ iwato: Iwato
\ jiao: Jiao
\ jiharkah: Jiharkah
\ karjighar: Karjighar
\ kijazKarKurd: Kijaz Kar Kurd
\ kumoi: Kumai
\ kurd: Kurd
\ leadingWhole: Leading Whole Tone
\ locrian: Locrian
\ locrianMajor: Locrian Major
\ lydian: Lydian
\ lydianMinor: Lydian Minor
\ mahur: Mahur
\ major: Major
\ majorPentatonic: Major Pentatonic
\ marva: Marva
\ melodicMajor: Melodic Major
\ melodicMinor: Melodic Minor
\ melodicMinorDesc: Melodic Minor Descending
\ minor: Natural Minor
\ minorPentatonic: Minor Pentatonic
\ mixolydian: Mixolydian
\ murassah: Murassah
\ mustar: Mustar
\ nahawand: Nahawand
\ nahawandDesc: Nahawand Descending
\ nairuz: Nairuz
\ nawaAthar: Nawa Athar
\ neapolitanMajor: Neapolitan Major
\ neapolitanMinor: Neapolitan Minor
\ nikriz: Nikriz
\ partch_o1: Partch Otonality 1
\ partch_o2: Partch Otonality 2
\ partch_o3: Partch Otonality 3
\ partch_o4: Partch Otonality 4
\ partch_o5: Partch Otonality 5
\ partch_o6: Partch Otonality 6
\ partch_u1: Partch Utonality 1
\ partch_u2: Partch Utonality 2
\ partch_u3: Partch Utonality 3
\ partch_u4: Partch Utonality 4
\ partch_u5: Partch Utonality 5
\ partch_u6: Partch Utonality 6
\ pelog: Pelog
\ phrygian: Phrygian
\ prometheus: Prometheus
\ purvi: Purvi
\ rast: Rast
\ rastDesc: Rast Descending
\ ritusen: Ritusen
\ romanianMinor: Romanian Minor
\ saba: Saba
\ scriabin: Scriabin
\ shang: Shang
\ shawqAfza: Shawq Afza
\ sikah: Sikah
\ sikahDesc: Sikah Descending
\ spanish: Spanish
\ superLocrian: Super Locrian
\ suznak: Suznak
\ todi: Todi
\ ushaqMashri: Ushaq Mashri
\ whole: Whole Tone
\ yakah: Yakah
\ yakahDesc: Yakah Descending
\ yu: Yu
\ zamzam: Zamzam
\ zanjaran: Zanjaran
\ zhi: Zhi
Which of the following forms should it be?
\aeolian: Aeolian'aeolian': Aeolianall = all.freezeAsParent;
I’m away from my pc at the moment (it’s late here), but I think this is the line that causes this. I think If you do Scale.all.parent you’d see all the entries.
@girthrub, I just approved your PR, looks like a clean fix! Don’t know if you want to wait for another review, but might be best to leave it for a couple of days so everyone gets to see it before merging.
Edit; and then I remember https://sc.dennis-scheiba.com/ @dscheiba!!! Yes that is why Scale.all looks empty, it’s only printing the first in the hierarchy. Why it doesn’t print all by default - I don’t really know!?
.all seems to be used only in connection with .put.
If that is the case, would it make sense to incorporate this behaviour directly into .put instead?
Then .all might not need to be documented separately.
indeed that was my hypothesis. I did test it was running with print, but further than that I was a bit confused on how to proceed…
it wasn’t before - we were able to address the various elements and get a list from there…
this is what it was iirc
Scale.aeolian
Scale.all[\aeolian]
These both work, so I don’t understand what the issue is here, aside from the fact printing Scale.all doesn’t work as expected?
@tremblap
Could you post an example of what you are trying to do?
I can swear that Scale.all[\aeolian] was not working… I was also trying Scale.at[\aeolian] which wasn’t working. And Scale.all is still empty, so this is where I got stalled…
Scale.all.aeolian this doesn’t work because the dictionary isn’t marked as ‘known’.
Scale.at[\aeolian] this won’t work because it expands to Scale.at(nil).at(\aeolian) - you probably meant Scale.at(\aeolian). The error message here is not useful!
Scale[\aeolian] this does not work because it’s trying to create a collection Array[1, 2]. Imo, this is confusing syntax and shouldn’t exist, but it’s quite wide spread.
Scale.[\aeolian] this does work. Seems like a crude fix for the above case, just confusing in my opinion.
Scale.aeolian works, goes through *doesNotUnderstand.
I don’t know if there is anything that can be done to make Scale.all show the entire hierarchy, I stay away hierarchies and Event prototyping because it confuses me. @jamshark70 I believe you have some experience with this, anyway to get posting to show the entire hierarchy?
Ehm…
I tested the following code in SC 3.9.3, 3.12.2, and 3.13.0:
Scale.all
In all of these versions it returns:
-> IdentityDictionary[]
The relevant part of the Scale.sc file hasn’t been changed for the last 13 years.
Also, the corresponding section of the help document hasn’t been changed for the same period:
method::all
The scale repository, to which new scales can be added.
code::
Scale.all.put(\catastrophic, Scale([0, 0.01, 0.04, 11.2]));
Scale.at(\catastrophic); // access the scale
::
From a normal user’s perspective (at least in my case), the common usage is simply Scale.aeolian.
For users who prefer the more “orthodox” style recommended in the help documentation, Scale.at(\aeolian) would be the alternative.
So in practice, there seems to be little reason to write Scale.all[\aeolian].
My understanding is that .all mainly exists to allow adding new scales, as shown in the help file.
So when I asked whether .all is only meant to be used with .all.put, I was thinking that it might be clearer if .put internally handled .all.
However, if there are a significant number of users who actually rely on Scale.all[\aeolian] instead of Scale.aeolian or Scale.at(\aeolian), then of course such a change wouldn’t be appropriate.
Also, I don’t see any particular reason why the behaviour of Scale.all would need to change.
Because when you ask for all, you expect it to return all of the scales.
There is already a PR related to this: Scale.directory, Tuning.directory: nicer formatting for output by HotwheelsSisyphus · Pull Request #7334 · supercollider/supercollider · GitHub
The text output will be slightly changed, but I think you will like it, as its readability is improved.
Ah…
I’m accustomed to it now, but it was definitely a hurdle for me when I first encountered .all and .directory.
It did feel rather odd at the time.
Perhaps it would be better if the functionality of .all and .directory were made more intuitive.
I mean, .all seems to provide the functionality of .directory, and .directory seems to provide the functionality of .all.
Tricky… By design, parent and proto entries are intended to be lurking in the background, available for lookup but not subject to primary dictionary operations.
a = (a: 1).parent_((b: 2));
a.removeAt(\b);
a[\b];
-> 2
removeAt doesn’t apply to parent entries, but .at(\b) = [\b] does look into the parent.
In many cases, this is a good thing:
p = Pbind(\a, 1).asStream;
e = p.next(Event.default);
-> ('a': 1)
e.postcs // oh wait, I can't do this... (don't try to do this)
I had wanted to show that all the parent keys would be displayed, but there’s an 8-year-old unfixed bug about a circular reference in the event data structure… so, never mind.
I guess one solution could be to create a subclass of IdentityDictionary that maintains the hierarchical structure but which operates as if it were flat (removeAt, keysValuesDo, printOn etc. would all behave as though everything were stored on the same level). This would accurately model the different requirements: for most dictionaries, you want the hierarchy, but in this case (and likely some others) you want a flat list, regardless of the storage structure.
(
~flatAsString = { |idict|
var postedKeys = IdentitySet.new;
var coll = CollStream.new;
var doComma = false;
var processOne = { |idict|
if(idict.size > 0) {
idict.keysValuesDo { |key, value, i|
if(doComma) { coll << ", " };
if(postedKeys.includes(key).not) {
coll << "(" <<< key << " -> "
<<< value;
doComma = true;
postedKeys.add(key);
};
};
};
};
coll << idict.class.name << "[";
processOne.(idict);
processOne.(idict.proto);
processOne.(idict.parent);
coll << "]";
coll.collection
};
)
~flatAsString.(Scale.all)
-> (too much output)
hjh
Thanks!
I guess one solution could be to create a subclass of IdentityDictionary…
Seems quite complicated just for printing. Unless it has other uses in the standard library.
I wonder if using parent here is even the correct solution? I guess it was done so the user can’t delete an existing scale, but they can override it with a new one, but why would you want to change the definition of a well established scale, its like changing the definition of ‘1’.
If it was redesigned, we could remove the public getter of all and have dedicated register and deregister methods Scale.register(\name, ...args to Scale.new ...) & Scale.deregister(\name).
I don’t see a good solution here.
The stated reason for the change was to “reduce garbage collection load,” but to be honest, I tend to agree with your point. SC’s general ethos for a long time has been, “we’re all grown-ups here, if you want to shoot yourself in the foot, at least in SC you won’t actually lose a toe.” The idea of protecting Scale definitions seems a bit contrary to that, and perhaps a bit over-engineered (while also being under-engineered, in that it didn’t consider the case of iterating over all scale definitions, for instance).
CPUs are pretty fast now. It may be better to drop the parent idea completely and just let it be a flat dictionary.
hjh