Deleting a branch is permanent. It CANNOT be undone. Continue?
#59 feature/improve_genconfig
Closed
LecygneNoir
wants to merge 0 commits from feature/improve_genconfig into develop
Reviewers
Labels
No Label
Milestone
No Milestone
Assignees
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
This pull request currently doesn't have any dependencies.
Loading…
There is no content yet.
Delete Branch 'feature/improve_genconfig'
This feature aims to fix #55 to have a working genconfig, and improve the genconfig process
Good overall. If I understand well we are not forced to do
python -m prismedia, I shouldn’t have changed all the README for the complex syntax when an easier one is here 😅.We may also use this function to ask if the user is sure it’s video is an mp4 file, maybe something for the
--with*options too like "Your configuration asks for a Thumbnail but none was found. Do you want to overight the configuration?".If you too think the function can be reused maybe we should place it in utils.py?
Also maybe rename it to "ask_overwrite" or "is_overight_authorized".
Good idea, it's done!
Even if you have a call to ask for the result, it is better to set the safest option here. And the safest is also the usual default for booleans.
I agree, but unforunately I find nothing elegant for that.
If we use False here, it means we have to duplicate the code to apply the option if there is no file. It force us to do "if overwrite then copyfile" and another "else then copyfile" after the if exists.
I try to deal with it but at the end, as the call is mandatory if file exists, I find better to not duplicate more code and simply sue True as default.
If you have a more elegant way to write it, it would be a pleasure!
In fact you next comments is the perfect solution 😍
I'll let you solve the thread if you think it ok? Or tell me if you see better alternatives ;-)
I would better have a "happy path" in the code with an "if exists(final_f) && not overwrite_or_not("..."): continue"
Okay! I am totally unfamiliar with this syntaxt of calling my own function inside a if (which is stupid from my brain, as
existsIS a function 🙄) but it's totally possible and THE elegant way to do I searched to avoid the "true by default" you noticed when reviewing 🎉Thanks for the suggestion!
Is it better for you like that?
Don’t you have to do "join(path, final_f)" too?
Maybe do the join before creating the final_f variable.
The join get the file from the "examples/xxxx" directory, so it's mandatory.
I have to create
final_fto remove the .samples from the examples/ directory, but I do not want to rename original files to avoid accidental overwrite if someone play with copy/paste manually 😅Perhaps I may create a original_f that contains both the directory and the file, but not sure it really worht it?
Maybe I am just confused about what the values of
path,fandfinal_fare.To me
fandfinal_fhold the same value, except for ".sample" that is removed fromfinal_f.fis a filename (no path) of successive file present in the "config" dir.pathcontains the path to the "config" dir.Oh ok, so you are doing what in shell would be
cp /path/f ./final_f. Doesn’t it assume the current path is the one of the program? Does it work if I start the program from any folder while "prismedia" is in the path?pathcontains the path to the module installed via pip concatened with "config", so when you install prismedia via pip, it get the file from inside the package, and it copy it to the local directory where you are running prismedia-init.As currently there is no "config directory" for prismedia, all config files are mandatory in the directory where you run prismedia, so it seems logical that genconfig create files here too.
genconfig already worked like that before, but should be reworked if we add a config directory option for prismedia I agree.
I agree that this is a change of behavior and we already have a plan for this one. We will do it later.
Thanks for the review!
In fact, the
python -msyntax is more for development point of view.I learn a bit about that when working on this feature with genconfig in fact :D
But basically, thanks to the
__init__.pyfile in the prismedia directory, python interpreter see it as a module, so when you clone the repository and go inside, by usingpython -m prismediayou tell python to use prismedia as a Module, and it takes first the module in your local directory.If you have no "prismedia" directory locally, then it's the module installed by pip that is used.
So you are able to test your code this way, without having to build a full package.
And by using
python -m prismedia.genconfigyou call the genconfig module inside prismedia, it's possible only if the module has a theif __name__ == '__main__':part inside.So you may use
python -m prismedia.utilsfor example, it'll call the file, but fail as there is nothing to launch inside.Thanks to the
[tool.poetry.scripts]in the pyproject.toml file, we are able to tell pip that the package provides theses scripts as shortcut to avoid thepython -msyntax for non-dev users.So I guess I'll indeed have to rollback that in README unfortunately, I didn't know that at the moment I reviewed your MR sorry :-/
Ok, still a bit fuzzy to me but I understood the idea. Don’t worry about the README, at least not in this PR.
Reviewers