Make XRD block natively multi-file#1868
Conversation
ml-evs
changed the title
There was a problem hiding this comment.
I have not personally tested the PR yet, though here are some of my first code review comments.
Some possible changes you could make, mostly instances where you could remove continue to better highlight the control sequence of the code.
A couple to do with defining variables such as pattern_dfs which is defined twice, once with a type and once without a type (one of them is redundant, my suggestion would be to move the one with a type to the place where the one without the type currently resides).
They are all minor changes to improve readability and remove a tiny bit of redundancy.
| ) | ||
| except Exception as exc: | ||
| warnings.warn(f"Could not parse file {f['location']} as XRD data. Error: {exc}") | ||
| continue |
There was a problem hiding this comment.
You could possibly remove this continue and add an else statement to the code after the try-except statement
| file_info = get_file_info_by_id(f, update_if_live=False) | ||
| except OSError: | ||
| LOGGER.warning("Missing file found in database but no on disk: %s", f) | ||
| continue |
There was a problem hiding this comment.
Could remove continue and put code you expect to run after a successful try except in an else statement after the try and except
| {"item_id": self.data["item_id"]}, | ||
| projection={"file_ObjectIds": 1}, | ||
| ) | ||
| pattern_dfs: list[pd.DataFrame] = [] |
There was a problem hiding this comment.
You define this later in the file. Either this is redundant or because it has a defined type needs to replace the later definition
| peak_data = {} | ||
| all_files = [{"location": filename, "immutable_id": filename} for filename in filenames] | ||
|
|
||
| pattern_dfs = [] |
There was a problem hiding this comment.
This was defined earlier... See previous comment
| y_options: list[str] = [] | ||
| for ind, f in enumerate(all_files): | ||
| try: | ||
| peak_data: dict = {} |
There was a problem hiding this comment.
Value {} for peak_data is never used. Possibly could remove and just have the type definition, etc: peak_data : dict, or you could put a type definition on the next line.
2 participants
Removes the bodged version of multi-file XRD we added with "All compatible files" -- ready to be improved in the future...