search

technosaby / RedHenAudioTagger

MIT License
1 stars 4 forks source link

1 understanding red hen existing pipelinecode audiopipe in hpc #19

Closed technosaby closed 2 years ago

technosaby commented 2 years ago
brucearctor commented 2 years ago

Suggesting that you add a .gitignore so not committing unneeded files ( ex: .DS_Stroe ).

brucearctor commented 2 years ago

It would be great ( bonus points, not required ). If we had instructions in a README for running locally.

What is needed for input -- any mp4 file? Other? ( I havn't actually read/run the code, yet ). These comments are more general than that at this point.

technosaby commented 2 years ago

It would be great ( bonus points, not required ). If we had instructions in a README for running locally.

What is needed for input -- any mp4 file? Other? ( I havn't actually read/run the code, yet ). These comments are more general than that at this point.

Yes I have added in the root folder, but it did not have the DS_Store file. So I modified it.

technosaby commented 2 years ago

It would be great ( bonus points, not required ). If we had instructions in a README for running locally.

What is needed for input -- any mp4 file? Other? ( I havn't actually read/run the code, yet ). These comments are more general than that at this point.

Yes I have modified the same. Can you please take a look ?

brucearctor commented 2 years ago

Looking much better.

For ease of anyone following [ I aim for it to be super easy for people to re-run what you've written and experience/see the same things. Think "reproducible research". For ex: is there a publicly available mp4 that is known to work well, that a user could either (a) be instructed to download manually via the README, (b) be provided a curl command to run to pull the file, or (3) have a script in this repo that pulls the file as a pre-/setup-step.

This entire comment block is certainly overkill for GSOC [ but supports good habits ], but is super nice if making it easy for others to follow along with the work [ meaning good README, and example/sample data ].

technosaby commented 2 years ago

Looking much better.

For ease of anyone following [ I aim for it to be super easy for people to re-run what you've written and experience/see the same things. Think "reproducible research". For ex: is there a publicly available mp4 that is known to work well, that a user could either (a) be instructed to download manually via the README, (b) be provided a curl command to run to pull the file, or (3) have a script in this repo that pulls the file as a pre-/setup-step.

This entire comment block is certainly overkill for GSOC [ but supports good habits ], but is super nice if making it easy for others to follow along with the work [ meaning good README, and example/sample data ].

Sure.. I have fixed this.

brucearctor commented 2 years ago

Minor nitpick --> See: https://peps.python.org/pep-0008/ and https://google.github.io/styleguide/pyguide.html#3164-guidelines-derived-from-guidos-recommendations ... [ ex: I see you use upper case filenames ]

technosaby commented 2 years ago

Minor nitpick --> See: https://peps.python.org/pep-0008/ and https://google.github.io/styleguide/pyguide.html#3164-guidelines-derived-from-guidos-recommendations ... [ ex: I see you use upper case filenames ]

I agree. As I work with other languages, I mix uped. Fixed!!!

brucearctor commented 2 years ago

Would add ... this is great to see PRs, as general workflow, and to see the changes that are meaningful for you to merge along the way. You don't need me to 'approve'/merge things like this - in your GSOC project. Ultimately is up to you to produce something meaningful, and you aren't touching an existing code base [ ex: larger open source project ] where we need those guardrails [ at this time ].