Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibility of reorgnizing the functions and fixing something seriously wrong? #7

Open
lvgeng opened this issue Jun 1, 2020 · 1 comment

Comments

@lvgeng
Copy link

lvgeng commented Jun 1, 2020

Sorry if I complaint too much, but I do think there are a lot of things need to be fixed.
Some obvious ones:

  1. The usage of the examples.
    For instance, the disparity sample. The documents says we should run
    python3 disparity_sample.py ~/path_to_file.xml -dmin 0 -dmax 10 -err False -scene 'real' -format tiff
    However, at least the "-err" is a problem cause the code requires "--err", and after that, the script would try to find a png file ~/path_to_file.png, which may not be generated if no adjustment is made to the code.
    To be honest, I have a strong feeling that some parts of the instrcution and testing samples are out of date because of the updating of the trunk part, would you mind to do some testing and fixing?

  2. missing functions
    For instance a function in disparity_methods.py, augment_costs_coarse() cannot be found defined.

  3. The structure of the code.
    It is just ... it feels different from a lib, it is more like a huge project for some specific things.
    For instance, the function rtxmain.estimate_disp(), it takes a lot of parameters (packed by a defined class) and returns a lot of thing (), while there is no comments to describe what they are exactly. (And... sorry for that but I am confused about the naming format you are using, a lot of variables begin with captical letters, which is kind of misleading while I try to understand the script)

Just saying, it is a good toolbox getting a lot of things done, but I really wish it is a little bit easier to use, like using a solid naming format, simple and specific functions dealing with different things respectively, and a function that using these functions to do the complex thing mentioned before rather than a giant function that does a lot of things at the same time (I am sure there is a good reason like that is the final goal or the same group of paremeters are used repeatly, but it is just so hard to understand what is going on or make any modification)

@freerafiki
Copy link
Owner

Don't worry about complaining, it is good for me.
You are totally right, it is actually a big project more than a library.
I agree with you on the changes, it's a matter of time. I could not find the time to fix all these things, which for the most case I am already aware of. I have some updated code also offline and wanted to create a documentation, but never managed. I don't see it happening this month, starting from next month it could be better and I hope I can do it. I will keep you updated. Also, if you have some changes that you see, you can correct and propose them (making a pull request) and I will check and incorporate into the code. That was also my hope for some stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants