MarcusBarnes / islandora_compound_batch

Provides the basic ability to batch import compound objects into Islandora.
GNU General Public License v3.0
3 stars 12 forks source link

Consider natural language sort order in create_structure_files #24

Closed whikloj closed 6 years ago

whikloj commented 6 years ago

I'm building some compounds and trying this out again. I noticed that because I use a numbering scheme to generate my sub-directories they fall out of order if there are more than 9 items. ie.

<islandora_compound_object title="pa_aar">
  <child content="pa_aar/1"/>
  <child content="pa_aar/10"/>
  <child content="pa_aar/2"/>
  <child content="pa_aar/3"/>
  <child content="pa_aar/4"/>
  <child content="pa_aar/5"/>
  <child content="pa_aar/6"/>
  <child content="pa_aar/7"/>
  <child content="pa_aar/8"/>
  <child content="pa_aar/9"/>
</islandora_compound_object>

Adding a natural language sort (or perhaps a configurable sort option) would help.

I just added this line

sort($stuffindirectory, SORT_NATURAL);

below here which results in my desired ordering.

<islandora_compound_object title="pa_aar">
  <child content="pa_aar/1"/>
  <child content="pa_aar/2"/>
  <child content="pa_aar/3"/>
  <child content="pa_aar/4"/>
  <child content="pa_aar/5"/>
  <child content="pa_aar/6"/>
  <child content="pa_aar/7"/>
  <child content="pa_aar/8"/>
  <child content="pa_aar/9"/>
  <child content="pa_aar/10"/>
</islandora_compound_object>
MarcusBarnes commented 6 years ago

@whikloj If I was to make the suggested change, would you be able to test the resulting pull-request? This would be easier since you already have material to test the change against.

whikloj commented 6 years ago

@MarcusBarnes I could certainly test it with my data, but it would be beneficial to have some other examples and how they currently operate. I don't want this to cause people's existing workflow to change if that can be avoided.

Even if we could brainstorm some example structures, I would be willing to try and write some simple PHPUnit tests to ensure stability.

MarcusBarnes commented 6 years ago

OK. Sounds good. @mjordan, do you think whikloj suggested change would cause any potential issues with how you use compound batch?

mjordan commented 6 years ago

No issues for me, thanks for catching this @whikloj.

cdeaneGit commented 6 years ago

Hi Jared,

BRAVO!!!!

cheers! cricket!

From: Jared Whiklo [mailto:notifications@github.com] Sent: Tuesday, February 27, 2018 10:23 AM To: MarcusBarnes/islandora_compound_batch islandora_compound_batch@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [MarcusBarnes/islandora_compound_batch] Consider natural language sort order in create_structure_files (#24)

I'm building some compounds and trying this out again. I noticed that because I use a numbering scheme to generate my sub-directories they fall out of order if there are more than 9 items. ie.

Adding a natural language sort (or perhaps a configurable sort option) would help.

I just added this line

sort($stuffindirectory, SORT_NATURAL);

below herehttps://github.com/MarcusBarnes/islandora_compound_batch/blob/master/extras/scripts/create_structure_files.php#L129 which results in my desired ordering.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/MarcusBarnes/islandora_compound_batch/issues/24, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALCX8naH-Pzn12anAc_PyUxKWiKZFTXYks5tZB3YgaJpZM4SVHnZ.

cdeaneGit commented 6 years ago

Hi Guys,

I have data I just ingested using my compound_prep.py script (https://github.com/cdeaneGit/compound_prep) to prepare the data for islandora_compound_batch, which worked perfectly.

I had my seed set to 1000 to make all the directories be numbered above 1000 and avoid the ordering problem, but I will reset to 0 and test the some of the same data with the latest version of islanodra_compound_batch on our test repository.

cheers! cricket!

From: Marcus Emmanuel Barnes [mailto:notifications@github.com] Sent: Tuesday, February 27, 2018 12:40 PM To: MarcusBarnes/islandora_compound_batch islandora_compound_batch@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [MarcusBarnes/islandora_compound_batch] Consider natural language sort order in create_structure_files (#24)

OK. Sounds good. @mjordanhttps://github.com/mjordan, do you think whikloj suggested change would cause any potential issues with how you use compound batch?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/MarcusBarnes/islandora_compound_batch/issues/24#issuecomment-368962450, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALCX8oNfrC-xpTPeNXe1s88vYD23s9BNks5tZD3TgaJpZM4SVHnZ.

MarcusBarnes commented 6 years ago

I'm glad to know @whikloj and @cdeaneGit that you're exploring or using this module. I hope to get some time to work on this tomorrow or Friday and then I'll submit a pull-request for testing and review.

whikloj commented 6 years ago

@cdeaneGit Ha I wish I had known about your script. I actually just wrote a python script for exactly the same purpose (https://github.com/whikloj/build_compounds). I'll have to peruse your repos.

MarcusBarnes commented 6 years ago

A pull-request is ready to be tested: https://github.com/MarcusBarnes/islandora_compound_batch/pull/25 Please add a comment to the pull-request the results of your testing. If we don't get any issues, @mjordan or myself will merge. Thank you in advance.

MarcusBarnes commented 6 years ago

Addressed in pull-request https://github.com/MarcusBarnes/islandora_compound_batch/pull/25 (merged with commit https://github.com/MarcusBarnes/islandora_compound_batch/commit/333874986577b773a07acde385effde93206db49). Thank you @whikloj @mjordan @cdeaneGit for your feedback regarding this issue.

cdeaneGit commented 6 years ago

I tested the new version vs. the old version on the same data that has 29 pages.

The sorting with the old was the “alphabetical-numeric” sort. The sorting with the new was straight numerical. Perfect.

Thanks for improvement!

cheers! cricket!

From: Marcus Emmanuel Barnes [mailto:notifications@github.com] Sent: Thursday, March 1, 2018 5:11 PM To: MarcusBarnes/islandora_compound_batch islandora_compound_batch@noreply.github.com Cc: Deane, Christine Haygood cdeane@utk.edu; Mention mention@noreply.github.com Subject: Re: [MarcusBarnes/islandora_compound_batch] Consider natural language sort order in create_structure_files (#24)

Addressed in pull-request #25https://github.com/MarcusBarnes/islandora_compound_batch/pull/25 (merged with commit 3338749https://github.com/MarcusBarnes/islandora_compound_batch/commit/333874986577b773a07acde385effde93206db49). Thank you @whiklojhttps://github.com/whikloj @mjordanhttps://github.com/mjordan @cdeaneGithttps://github.com/cdeanegit for your feedback regarding this issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/MarcusBarnes/islandora_compound_batch/issues/24#issuecomment-369748730, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALCX8hq3EZtnFvmvYIOrgoezJzbk5_O8ks5taHH1gaJpZM4SVHnZ.