jeanluct / braidlab

Matlab package for analyzing data using braids
GNU General Public License v3.0
23 stars 9 forks source link

Input argument parsing #80

Open jeanluct opened 9 years ago

jeanluct commented 9 years ago

(From Marko)

As use-cases for different functions, e.g., entropy.m, it becomes more and more cumbersome to manually check and update their argument list. We should convert these more complicated functions to use inputParser.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-10-22 20:07:09+00:00

Absolutely. The current setup is a mess.

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-22 20:45:50+00:00

entropy.m now supports different length functions; slight change in default arguments - tol = [] now implies tol = default, not tol = 0 as before. This was done to allow skipping it. For a long-term solution see issue #80

→ <<cset 327ddecf0e91>>

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-30 23:05:39+00:00

I have just moved entropy to inputParser ( ea5ea27 ). This was my first approximation to how things could look like. 2b13736 introduced a function that can be used to specify multiple names for a single flag, e.g. "bh", "trains", etc. for train tracks. It works by partial matching too, so it's enough to write "tr" to match "train-tracks" for example.

jeanluct commented 9 years ago

Should this be closed?

mbudisic commented 9 years ago

entropy and complexity now have the new interface but this is definitely not implemented across the board. I doubt that we want to keep the full update of all the functions as "release-2.2" milestone, but perhaps it would be good to keep it up as a ticket.

jeanluct commented 9 years ago

Ok, deleted the milestone.

jeanluct commented 9 years ago

I guess this issue is still open? The constructors for braid and loop, for instance, are still mess. In fact they should probably be delegated to helper functions, to declutter things a bit. Anyways, this isn't very urgent.

mbudisic commented 8 years ago

OK, I did a bunch of work in iss80 branch on braid interface. There are several major novelties:

All unit tests pass, except for taffy, for which databraid complains that the crossing times are not appropriately ordered. Could you please run the unit tests and see if this is a valid concern or not?

I didn't merge yet, because this is a pretty big change, and I wanted to see what's your opinion. (There is also a bit more work to be done, e.g., making sure documentation reflects the changes)

jeanluct commented 8 years ago

I'm not so happy about your change to the projection angle... it may be tedious but it is "internal". Isn't there a way to do this fairly cleanly?

Please don't merge into the develop branch until we test all this.

mbudisic commented 8 years ago

There is no way to do this cleanly - MATLAB's inputParser detects an argument based on its position and the argument's type (but not on type of some other argument). I still did it, since you seem to use the old interface often. It's kludgy but it's better than it was before (I think).

So, I reverted tests to the old interface - everything should work as it used to be. Errors are slightly different (the "addCause" allows you to describe what had happened, e.g., "badarg", and what caused that bad arg - e.g., an error deep in the sub-sub-invocation of processing that argument). This way we don't have to reinterpret the lower-level errors at every higher level to give them proper tags. (It results in a slightly more complicated unit test verifyError function, but that's it).

Still, I am not merging until we both agree with these changes.