Renaming Run to Shot #85
Conversation
Resolves Rename Run to Shot labscript-suite#81 Renamed classes/arguments/docs from run to shot and added dprecation notices where appropriate. Arguments (distinct from keyword arguments) were renamed without worrying about backwards compatibility under the assumption that people are not using them as keyword arguments (which is technically possible).
|
@zakv Would also appreciate your eyes on this (don't seem to be able to explicitly request a review from you using the GitHub interface since you are not a project member. Might have to fix that) |
There was a problem hiding this comment.
Overall looks good! I made a few comments, and have a couple of overall questions.
- Is there any particular reason for using double underscores rather than single? It's not clear to me how the name mangling helps.
- Why is "Shot" better than "Run"? I agree that normally I'd use the word "shot" (in fact our lab uses a custom
Shotclass than inherits fromlyse.Run) but I would think that "run" would mean the same thing. Is the word "run" misleading somehow? I agree that iflysewere being written from scratch I'd rather name this classShotthanRunbut I'm not 100% convinced it's confusing enough to be worth making users update all of their analysis scripts and other code.
|
Also, Edit: |
* split deprecation messages across multiple lines * Fixed deprecation message punctuation * Fixed missing property decorator in `Sequence` class * Updated docs to use `Shot` class.
In this instance I did it to made the variable read only. Not strictly necessary, but there it's really an internal variable and it's better practice to keep it from being modified outside of the class. It's basically just defensive programming, and makes it easier for internal changes to be made down the line without worrying about whether we're breaking the API.
Run could be interpreted as a set of shots (a "run" of shots"). Run was very early nomenclature we adopted, but it rapidly become confusing when used in every day speech. We thus settled towards the more obvious "Shot" and "Sequence" which cannot be confused. We were hesitant to change the API at the time, but I think it's probably time to bite the bullet and do it. I agree it's a bit of a pain to change code, but |
Good to know, thanks! I wasn't sure if there was something that the double underscore name mangling signaled to users that a single underscore didn't.
To be honest our lab used to use "shot" and "sequence" interchangeably, haha. That was back when we used a labview-based control system where you designed shots by listing a sequence of output times/voltages in a table. I suspect though that the large majority of people would interpret "shot" and "sequence" in the way you mentioned. All your changes look good from my perspective. The PR looks ready to merge as far as I can tell. |
…eprecation status.
Resolves #81
Renamed classes/arguments/docs from run to shot and added dprecation notices where appropriate.
Arguments (distinct from keyword arguments) were renamed without worrying about backwards compatibility under the assumption that people are not using them as keyword arguments (which is technically possible).