Aut: Improve WacArcInputFormat.java test coverage

[ruebot]

WacArcInputFormat.java test coverage can be improved.

[ruebot]

Partially resolved with: https://github.com/archivesunleashed/aut/commit/23d0cb4029896e82a5c4af01cf270894a302bf0b

[greebie]

It appears to me that this + #78 can be resolved by removing the files, in favor of the Generic class. I defer to @ruebot on this though. I do not see where it is currently being used after https://github.com/archivesunleashed/aut/commit/5e99d63083ac0b3c13011a528f247dc47ed66004

[ruebot]

[nruest@gorila:aut] (git)-[issue-80]-$ grep -R "WacArcInputFormat" .
./src/main/java/io/archivesunleashed/mapreduce/WacArcInputFormat.java:public class WacArcInputFormat extends FileInputFormat<LongWritable,
./src/test/java/io/archivesunleashed/io/ArcRecordWritableTest.java:import io.archivesunleashed.mapreduce.WacArcInputFormat;
./src/test/java/io/archivesunleashed/io/ArcRecordWritableTest.java:        WacArcInputFormat.class, conf);
./src/test/java/io/archivesunleashed/mapreduce/WacArcInputFormatTest.java:public class WacArcInputFormatTest {
./src/test/java/io/archivesunleashed/mapreduce/WacArcInputFormatTest.java:        ReflectionUtils.newInstance(WacArcInputFormat.class, conf);
Binary file ./.git/index matches

[nruest@gorila:aut] (git)-[issue-80]-$ grep -R "WacWarcInputFormat" .
./src/main/java/io/archivesunleashed/mapreduce/WacWarcInputFormat.java:public class WacWarcInputFormat extends FileInputFormat<LongWritable,
./src/test/java/io/archivesunleashed/io/WarcRecordWritableTest.java:import io.archivesunleashed.mapreduce.WacWarcInputFormat;
./src/test/java/io/archivesunleashed/io/WarcRecordWritableTest.java:        WacWarcInputFormat.class, conf);
./src/test/java/io/archivesunleashed/mapreduce/WacWarcInputFormatTest.java:public class WacWarcInputFormatTest {
./src/test/java/io/archivesunleashed/mapreduce/WacWarcInputFormatTest.java:        ReflectionUtils.newInstance(WacWarcInputFormat.class, conf);
Binary file ./.git/index matches

That's what I got. If you want to proceed with the refactoring, go for it.

[greebie]

Looks like they can go. I'll check these today and see what blows up. I'll also change the "generic" formats to just plain archives.

[greebie]

The only risk here is that, if all works out, we would remove .arc and .warc specific loading functions from the code without a deprecation warning. This is already technically the case with the changes to RecordLoader in 0.11 anyway, but still. Just a head's up @lintool @ianmilligan1 !

[ruebot]

We don't have a 1.0.0 release, so we don't have to do it. But, I think a deprecation cycle would be helpful. Is this something you want to take on?

[greebie]

Sure thing! I meant to assign this to myself earlier.

Draft plan as follows:

1. Create a branch and test changes removing references to LoadArcRecord and LoadWarcRecord, and associated interfaces and classes.
2. Change "Generic{x}" to {x}
3. Establish whether deprecation cycle is necessary -- (is anyone using these now that we have LoadArchive -- I don't believe they are even in the documentation anymore)?
4. Create PR when ready.

[ianmilligan1]

I go back and forth on the deprecation cycle. My sense is that if a user if engaged enough to see a deprecation cycle – i.e. they're checking out a new release – they've noticed the change in all our docs, etc. that we've made since February 2016 when this new functionality came in.

[greebie]

I think I agree here. It would be good to establish deprecation procedures generally in future, but we may be able to get away with no deprecation in this particular case. For example, do you want to deprecate WriteGDF in future?

[ruebot]

In Islandora and Fedora, we would do one release cycle. So, we'd identify something that need to be deprecated, and wrap it in a deprecation warning. Then release, and it would be in the release notes. Then after that we'd remove it.

[ruebot]

A bit dated, but relevant: https://docs.oracle.com/javase/1.5.0/docs/guide/javadoc/deprecation/deprecation.html

[greebie]

Okay - so two alternatives.

1. Remove from 1.0, no deprecation warnings, but include notice in release notes.
2. Keep in branch for now, add deprecation warnings to 1.0 including warning of removal in next release, create PR and merge for 1.1.

There would also be a number of "Generic_._" items deprecated, none of them for front-end usage. If we kept deep documentation on the library I would see the deprecation warnings as more important, but I'm good with either option.

If we go with option #2, I will take responsibility for adding the deprecation comments to the code for 1.0. The only two user-relevant deprecations (imho) are RecordLoader.loadArcRecord() and RecordLoader.loadWarcRecord()

[ianmilligan1]

OK thanks all – my own vote is to go with option 2 then, it'd be good to get in the habit of doing this.

And yes, my gut says maybe we should deprecate WriteGDF too. Although you're the networks scholar.. can you think of any cases where somebody might want a GDF instead of a GEXF?

[ruebot]

What's 1.0 and 1.1? Our current release is 0.11.0, and we use semantic versioning.

[greebie]

It appears that https://github.com/archivesunleashed/aut/commit/5e99d63083ac0b3c13011a528f247dc47ed66004 already removed the associated commands. I also take note the comment by @lintool here as related to java files: https://github.com/archivesunleashed/aut/issues/102#issuecomment-340195847 .

I pushed a new branch "LoaderRefactor." It has been tested and works for a minimum of 4 scripts in the documentation.

[ruebot]

Partially resolved with: https://github.com/archivesunleashed/aut/commit/47b5d26240a7771242ca1ae36a05a03bf19f7f92

[greebie]

Code deprecated and will be removed in a later release. Closing.

[ruebot]

Fully resolved with: https://github.com/archivesunleashed/aut/commit/8fb766eee86001f4545d38e1211a97057717e740