PFython
commented
3 years ago Hi Quentin - I've recreated your lastest README in this Pull Request so you should be able to Merge now without any conflicts 👍
Closed PFython closed 1 year ago
PFython
commented
3 years ago Hi Quentin - I've recreated your lastest README in this Pull Request so you should be able to Merge now without any conflicts 👍
qmasingarbe
commented
3 years ago Hi @PFython , thank you for your pull request. Here are my thoughts:
TICKS_PER_SECOND: not sur about this one as it seems that unit like kilometres per hour or metres per second are written this way...
list_clips: not sure about this one neither, I understand than list_video is a bit broad but list_clips is misleading as it doen't list audio clips. If I had to change it I would probably go with list_video_clips. Anyway changing it will probably be a bad idea because it will break compatibility with previous versions...
wording of comment/print: perfect
demo_batch2: The main purpose of the demo_batch is to display the capabilities of pymiere to act without a human, to be batch processed on a render farm or something similar. Adding UIs kind of defeat this purpose. However I like the idea of the having another UI example. Qt is a professional level and industry standard for UIs but it can be a bit complicated for newcommers. Let's keep both of the demo_batch and demo_batch2 files. Note that your demo_batch2 is not compatible with python 2 due to the use of pathlib (although py2 is deprecated a lot of companies still use it due to legacy code). As it is focused on newer python user it is not a problem as long as it is mentionned in the py.
demo_ui: Be carefull as it seems that the pull request will revert some of my fixes in the demo_ui.py
BigRoy
commented
1 year ago Should this PR be closed in favor of certain decisions - in the current state at least is has some conflicts?
TICKS_PER_SECOND does sound better to me than TICKS_PER_SECONDS but it'd just be breaking backwards compatibility. Technically we could define both for now and deprecate one - but I'm not sure it's enough to warrant breaking older code at all.
Looking at where we're at it might still be nice to refactor some wording in comments that we feel is way off or just plain typos but basically leave the rest as is?
A couple of suggestions to variable names (TICKS_PER_SECOND and list_clips) Suggested changes to wording of print() output Suggested changes to wording of comments A new (replacement?) suggestion for demo_batch called demo_batch2 using PySimpleGui (more accessible for new users who may be unfamiliar with Qt5).