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

Attempted fix for True Append code #106

Merged
merged 3 commits into from
Jul 16, 2024
Merged

Conversation

niemasd
Copy link
Collaborator

@niemasd niemasd commented Jul 16, 2024

The previous version of the True Append code created a temporary file in order to perform the pairwise "new vs. old" TN93 calculations. However, @stevenweaver reported the following:

It looks good, but I'm encountering an occasional race condition with the temporary file creation here. I verified this by inserting a breakpoint one line prior to the relevant code, then continuing execution. The issue disappears when using the debugger (pdb) but reoccurs under normal execution.

This PR consists of 3 commits:

  1. 95e8823 replaces the temporary file (which is what was causing the aforementioned issue) with a named pipe (FIFO)
    • This should also slightly speed up the code, as it removes unnecessary disk accesses for writing/reading the temporary file
    • However, while this update no longer uses tempfile.NamedTemporaryFile, it now uses tempfile.mkdtemp to create a temporary directory within which to create the named pipe, so it might still have the same issue as before (@stevenweaver please test it out to check)
  2. 70ebec1 refactors the true_append function and removes the main function to reduce redundant code
    • In the original True Append script, a function called main was executed when the script was run from the command line, and this main function would (1) use argparse to parse the user's command line arguments, and then (2) orchestrate the execution of the actual True Append functions
    • When the True Append script was migrated to the hivtrace repository, a new function called true_append was created as a modified copy of main but which parses the relevant inputs via function arguments rather than command line arguments
    • This commit refactors true_append to parse the relevant True Append inputs via function arguments if they're provided, otherwise it will try to parse them from the command line, removing the need for a separate main function
  3. 4d79020 adds the Python 3 shebang to the other Python scripts in this repository, and it makes the scripts executable (chmod a+x)

@stevenweaver stevenweaver merged commit 15b04e2 into veg:true-append Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants