PhonologicalCorpusTools / SLPAA

5 stars 0 forks source link

137 location module add default neutral option #329

Closed gracemyz closed 3 months ago

gracemyz commented 5 months ago

Implement a default neutral checkbox. When checked, add default neutral paths to the location module.

This particular request is not done: "The locations for 'default neutral' should be similar to those for 'handedness' (right-hand / left-hand). That is, there should be a global setting option, but also each individual signer can be marked as having their 'own' default neutral specification. So, when a signer is selected in Sign-Level Information (this hasn't been implemented at all yet), the location module will inherit the specification from the individual."

kvesik commented 4 months ago

@gracemyz When I create a new location module, it doesn't matter whether I set the articulator to Hand 1, Hand 2, or Both hands-- in all cases when I push the button to Apply 'neutral' settings, it sets the 2-handed setting (chest/breast area).

Specific context: in settings, I changed the "default neutral two-handed location" to be body-anchored (specifically, chest/breast area) and left the "default neutral one-handed location" as is (spatial / ipsi-mid-med).

gracemyz commented 4 months ago

@kvesik The 1h/2h setting is determined by the Sign type setting (if not specified, the default is two-handed). We're not actually checking which articulator is specified at all. (I'll add an info box to make this clear to the user!)

There's a possible conflict where Sign type is specified as 1h, but then articulator is specified as "Both hands". Currently we allow this to happen without giving any warnings. Should we put in some checks for this? Maybe it should be addressed in a separate issue?

kvesik commented 4 months ago

Oh right! Thank you @gracemyz for that reminder. But yeah, would be curious to know what @kchall thinks about that possible conflict between sign type and articulator selection.

kchall commented 4 months ago

@gracemyz , @kvesik Looks great! Could we add one other checkbox option in the "Preferences / Location" dialog box, where it has the checkbox for "Automatically select 'This location is neutral' when "Apply neutral settings" button is pressed" that says "Automatically select 'This location is neutral' when "Default neutral location" is selected." (And then implement this -- "Default neutral location" is a list option under the "Purely spatial" signing space options, as shown below. I do think we should keep this as yet another way for someone to designate an underspecified neutral space instead of deleting it now that we have the other options, but it should tie into this functionality.)

image image
kchall commented 4 months ago

@gracemyz Lol, I too got foiled by the tie-in to "Sign Type" instead of "Hand 1 / Both hands" in the module! Seems to be working just fine when I follow my own instructions. Sorry.

But maybe this means we need a tool tip over the "Apply neutral settings" to remind the user that (1) these can be adjusted in the Preferences / Location menu and (2) that they will be determined based on the specification in the Sign Type module? Thanks!

gracemyz commented 4 months ago

@kchall Please take a look at the updated branch! I've added the new autocheck option and an info box. image image

kchall commented 4 months ago

Super, thanks, @gracemyz ! This all looks good, though because of the difference in Macs and PCs, I think we should probably modify the wording in the "Info" box both times to just say "Preferences > Location" (even though this doesn't tell people where "Preferences" is located)...it's just that it will only say "Settings" on a PC, and I think it's likely better to underspecify the location of "Preferences" than to actively tell Mac users to look for something that isn't there.

Also, something has now gone wrong when I try to change the default neutral specifications; I can't change anything, and instead get the following error:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/idlelib/run.py", line 156, in main
    ret = method(*args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/idlelib/run.py", line 559, in runcode
    exec(code, self.locals)
  File "/Users/KCH/Desktop/SLPAA/src/main/python/main.py", line 8, in <module>
    exit_code = appctxt.run()
  File "/Users/KCH/Desktop/SLPAA/src/main/python/gui/app.py", line 23, in run
    return self.app.exec_()
  File "/Users/KCH/Desktop/SLPAA/src/main/python/gui/main_window.py", line 806, in on_action_edit_preference
    pref_dialog.exec_()
  File "/Users/KCH/Desktop/SLPAA/src/main/python/gui/preference_dialog.py", line 512, in <lambda>
    self.defaultneutral_2h_button.clicked.connect(lambda: self.change_default_neutral(2))
  File "/Users/KCH/Desktop/SLPAA/src/main/python/gui/preference_dialog.py", line 525, in change_default_neutral
    self.locationselector = DefaultNeutralDialog(numhands, parent=self)
  File "/Users/KCH/Desktop/SLPAA/src/main/python/gui/preference_dialog.py", line 565, in __init__
    self.locationoptionsselectionpanel = LocationOptionsSelectionPanel(treemodeltoload=self.currenttreemodel, displayvisualwidget=False, parent=self)
  File "/Users/KCH/Desktop/SLPAA/src/main/python/gui/locationspecification_view.py", line 75, in __init__
    self.dominanthand = self.mainwindow.current_sign.signlevel_information.handdominance
AttributeError: 'NoneType' object has no attribute 'current_sign'

(It's the same error for both the one- and two-handed options.)

gracemyz commented 4 months ago

Sorry about that - I introduced some bugs when I merged from main. Changed to Preferences>Location, and also updated to include Kaili's location images.

kvesik commented 4 months ago

@gracemyz @kchall updated default neutral behaviour looks good to me!

One concern about layout-- on my screen, the location module is now too tall to fit on the screen in its entirety, and since it can only be dragged via the top bar, it's difficult to move it into a position where the buttons at the bottom are visible. Maybe tomorrow at the meeting we can discuss options to make the layout a bit less vertical? I can think of a few off the top of my head...

Option 1: (For all modules) move the "phonetic" location checkbox so it's next to the "phonological" one instead of below.

image

Option 2: (For just the location module) rearrange the default neutral buttons onto two rows instead of three.

image

Option 3: (For just the location module) reduce the amount of space that the bottom panel is allowed to take (reduce scale of image as well as max heights of location list & location details table). image

Option 4: (For all modules) add scrollbars to module specification dialogs, with a max size on the dialog.

kchall commented 4 months ago

@gracemyz This all looks great -- I think it's ready to merge in to Main, thank you!