Add batch processing to 'draft' action, add test artifacts for zero, … #54
Conversation
pds_doi_core/actions/draft.py
Outdated
| """Function receives an input which can be one name or a list of names separated by a comma or a directory. Function will return a list of names. | ||
| """ | ||
| o_list_of_names = [] | ||
| if ',' in input: |
There was a problem hiding this comment.
I think we can have the same code for a list of pds4 labels or a single one. If you split on ',' and there is no ',' you get one list with a single result.
Then you can test if the token is a dir or a file.
There was a problem hiding this comment.
You are right. I started with one train of thought and kept going.
Splitting using the comma ',' right away allow you to use the for loop to check if the token is a directory or a file.
Code is much simpler now.
pds_doi_core/actions/draft.py
Outdated
| # -i , | ||
| if len(token) > 0: | ||
| o_list_of_names.append(token) | ||
| elif os.path.isdir(input): |
There was a problem hiding this comment.
This elif and the following else would be in the for token loop. Each step will append (file) or extend (dir) the list with new entries.
Does that work ?
There was a problem hiding this comment.
Putting the elif and the following code inside the for loop works now.
pds_doi_core/actions/draft.py
Outdated
| list_of_names = self._resolve_input_into_list_of_names(input) | ||
| # Return immediately if there are no files found. | ||
| # Note that exit() function is not used as it would interfere with the unit test. | ||
| if len(list_of_names) == 0: |
There was a problem hiding this comment.
Can that happen ? if the argument input is mandatory then there is at least one input. You should be able to remove this test and the lines inside it.
We could have an un-existing input or empty input but that is not what you are testing here.
There was a problem hiding this comment.
Yes, it can happen if the input for the -i is a directory and there is nothing inside. As it turns out, we don't need to do the check since the while loop will take care of that (iterating over a list of zero).
I removed len() check and the code still runs OK. You are right. There's no need to do an additional check when the for loop works just fine.
pds_doi_core/actions/draft.py
Outdated
| # Write a transaction for the 'reserve' action. | ||
| transaction_obj.log() | ||
| # The returned label can be None if it is not an expected XML file. | ||
| if doi_label: |
There was a problem hiding this comment.
This test looks like an implicit error management. That would be more readable to have an explicit exception (invalid PDS4Input...) raised by the method _transform_pds4_label_into_osti_record().
But
I don't know what case would raise this exception. Do you know what case can make the doi_abel == None ? Ah yes maybe if the input file does not exists... yes in this case an exception will be more explicit.
There was a problem hiding this comment.
To avoid having to check for doi_label be None, the function _transform_pds4_label_into_osti_record() can return an empty tree (which is not None). There's no need to check for None-ness.
The code is much simpler now.
…one and two files directory, expand unit tests, fix parsing of 'first_name' when splitting with space fail Simplify function parsing for input files/directories, delete unit test for empty directory
Pull request to close DOI Batch Processing capability #25
Batch processing allow to process series of PDS4 labels.