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

ImageJ plugin - Group files Exception #1967

Merged
merged 5 commits into from Sep 30, 2015

Conversation

dgault
Copy link
Member

@dgault dgault commented Sep 8, 2015

ImageJ plugin - Exception when both 'group files' and 'open individually' options selected

A test case and steps to reproduce are linked from the ticket.
https://trac.openmicroscopy.org/ome/ticket/12941

Splitting from #1964

Issues included:

  • IndexOutOfBoundsException when both options selected in Main Dialog
  • For certain file formats the regular expression and 'file contains'
    file patterns were not being implied
  • Entering incorrect values into the Dimensions setting of the
    FilePatternDialog resulted in a 'File is not of a supported format'
    error

Solution:

  • IndexOutOfBoundsException resolved in ImportProcess by reading the
    group files option. When both options are selected then group files is
    not automatically the inverse of ungroup files
  • For file formats that have a fileGroupOption of FormatTools.MUST_GROUP
    the second FilePatternDialog is skipped and replaced with an ImageJ
    message box informing the user why file pattern options are not
    available, stating "Image specifications state that files of this given
    format cannot be handled separately and must be grouped according to
    supporting metadata". A new importer option flag was added to test and
    store the must group condition

…lly' options selected

A test case and steps to reproduce are linked from the ticket.
https://trac.openmicroscopy.org/ome/ticket/12941

Issues included:
- IndexOutOfBoundsException when both options selected in Main Dialog
- For certain file formats the regular expression and 'file contains'
file patterns were not being implied
- Entering incorrect values into the Dimensions setting of the
FilePatternDialog resulted in a 'File is not of a supported format'
error

Solution:
- IndexOutOfBoundsException resolved in ImportProcess by reading the
group files option. When both options are selected then group files is
not automatically the inverse of ungroup files
- For file formats that have a fileGroupOption of FormatTools.MUST_GROUP
the second FilePatternDialog is skipped and replaced with an ImageJ
message box informing the user why file pattern options are not
available, stating "Image specifications state that files of this given
format cannot be handled separately and must be grouped according to
supporting metadata". A new importer option flag was added to test and
store the must group condition
…iles

New functionality should be:
Neither option selected - Open group files - No group files dialog
Open individually selected - Open individually
Group files with similar names selected - Open group files - message
popup instead of group dialog if must group
Both options selected - Open group files - message popup instead of
group dialog if must group
@jburel
Copy link
Member

jburel commented Sep 13, 2015

  • The files attached to the e-mail sent by Joaquim now import correctly. The 2 files are considered and the listed as 2 series.
    A dialog pops up when the "Group files by similar names" is selected. Is that still expected?
    This is in the only case it happens,
  • using showinf
    Reading core metadata
    filename = /Users/jmarie/Desktop/002002001.flex
    Used files:
    /Users/jmarie/Desktop/002002001.flex
    /Users/jmarie/Desktop/002002002.flex

@dgault:
in the description, we also usually indicate steps required to test the PR
e.g.

  • Select image A
  • Check the following options in dialog

@dgault
Copy link
Member Author

dgault commented Sep 14, 2015

@jburel
Firstly, my apologies for not fully including testing steps.

  • The first point regarding the dialog is I guess more of a user experience discussion point. The popup when "Group files by similar names" is selected is the expected outcome in this case (when the files used have been marked as 'Must Group').

The question is whether or not it is better to have that popup informing the user why the file stitching options are not being presented as normal, or if the popup is too intrusive then perhaps it would be best to leave it out all together and go straight to the series options screen. I've attached an image below to clearer demonstrate the 2 possible options. If there is a preference for the second option then the PR can be easily updated.

pr1967

  • The second point regarding the filenames used I will look into and update if required

@jburel
Copy link
Member

jburel commented Sep 14, 2015

@dgault: no problem at all, The description of the PR was already well documented

@jburel
Copy link
Member

jburel commented Sep 14, 2015

Having an extra window is probably not a bad thing and does not break the "usual" ImageJ workflow (several windows can pop up when performing a task e.g. export as OME-TIFF)
The display of the text should be improved.

Changing text to make it clearer to the user that File Stitching Options
are being skipped
@dgault
Copy link
Member Author

dgault commented Sep 16, 2015

Updated the text being displayed to try and make it clearer to the user that the file stitching options are not available for the selected files.

@joshmoore
Copy link
Member

Does this need to be relisted?

@jburel
Copy link
Member

jburel commented Sep 22, 2015

I was planning to do it yesterday afternoon
I will take care of it either today or tomorrow for a final check

@jburel
Copy link
Member

jburel commented Sep 25, 2015

  • Typo: Stitiching => Stitching
  • \n should be after format

Fixed spelling mistake and inserting new line for second sentence
@jburel
Copy link
Member

jburel commented Sep 30, 2015

Text display is improved. typo fixed. Just noticed a missing dot at the end of the second sentence.
When this is fixed and travis is happy, it will be ready to be merged.

Missing full stop added
@jburel
Copy link
Member

jburel commented Sep 30, 2015

@dgault thanks. Good to merge

melissalinkert added a commit that referenced this pull request Sep 30, 2015
ImageJ plugin - Group files Exception
@melissalinkert melissalinkert merged commit f3e75ee into ome:develop Sep 30, 2015
@sbesson sbesson added this to the 5.1.5 milestone Oct 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants