Opened 9 years ago

Closed 9 years ago

#391 closed defect (fixed)

cannot use "sides" option of command "cartoon style"

Reported by: Elaine Meng Owned by: Conrad Huang
Priority: major Milestone:
Component: Depiction Version:
Keywords: Cc:
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

"Cartoon style" usage info limits sides to 3-10 but clearly can be set higher with original "car param" command and the default for round/oval is already >10, it is 12. Currently there is an error if you try to use sides in the style subcommand, despite it being shown in the usage:

cartoon style sel xsection barbell barScale .9 sides 10
Traceback (most recent call last):
File "/Users/chimera/Applications/ChimeraX_Daily.app/Contents/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/chimerax/cmd_line/gui.py", line 168, in execute
cmd.run(cmd_text)
File "/Users/chimera/Applications/ChimeraX_Daily.app/Contents/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/chimerax/core/commands/cli.py", line 2194, in run
results.append(ci.function(session, kw_args))
TypeError: cartoon_style() got an unexpected keyword argument 'sides'

Apparently there is some confusion with the sides option of cartoon tether. Maybe change the cartoon style keyword "sides" to something else, say divisions or subdivisions?

Change History (7)

comment:1 by Conrad Huang, 9 years ago

Resolution: fixed
Status: newclosed

Fixed in bef4ccb.

There was a conflict because command registration called the option "sides" but the function used "bar_sides". I changed the registration to "bar_sides" (or "barSides" if you prefer) to match "bar_scale".

comment:2 by Elaine Meng, 9 years ago

My intent was that the keyword would apply to both the oval (round) and barbell (piping) cross-sections.  Do you feel strongly about having two separate parameters?

comment:3 by Conrad Huang, 9 years ago

Resolution: fixed
Status: closedreopened

Oh, I see. It only applies to the barbell xsection right now. I don't really have an opinion either way. My only comment is that if we always use the same parameter for both, a barbell cross section will be using twice as many sides as an oval cross section. If that's acceptable, I have no problems with changing the option back to "sides" and making it apply to ovals too.

comment:4 by Elaine Meng, 9 years ago

I don’t get why that would make barbell have twice as many sides as oval for a given value of the parameter, but accepting that, the “sides” name might be misleading.

So if you are OK with a single keyword setting for both barbell and oval, I see two choices:

(1) continue to call it “sides” but internally have two parameters and divide by two for the barbell parameter so that “sides N” gives the actual number of sides for both barbell and oval.  

(2) call it something else like “subdivision” and just document that “subdiv N” will give N sides for round and 2N sides for barbell. This might be more appropriate considering that the barbell is a more complex shape and would “need” more sides for a given level of zoom to look good.

However, I’m OK with either one of those… what do you think?

comment:5 by Conrad Huang, 9 years ago

I was describing how the current implementation interprets "sides" for the different cross sections. Right now, it is used as the number of sides for the circular parts. So for oval, there's obviously only one, but for barbell there are two (one on either side). I can change that so that for N sides, ovals have N sides; and barbell will have (N-2)/2 sides for each "bell" and 2 for the "bar". Then a single parameter is consistent for both cross sections. Then we can keep the "sides" name (which I like) rather than "divisions" or "subdivisions". Does that sound reasonable?

comment:6 by Elaine Meng, 9 years ago

That sounds reasonable. If I understand correctly, to a user it is the same as my choice #1, with some magical Conrad stuff going on internally.  :-)   Thanks!

comment:7 by Conrad Huang, 9 years ago

Resolution: fixed
Status: reopenedclosed

Fixed in 69737fb.

The "sides" option in "cartoon style" now applies to both oval and barbell cross sections. The value is the total number of sides in the cross section. If the number of sides is odd, then the barbell cross section will have an extra side since it is symmetric and the number of sides must be even.

Note: See TracTickets for help on using tickets.