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

Use lowercase name for package #54

Conversation

ajnelson-nist
Copy link
Contributor

This patch addresses a runtime issue not found in PR 46.

This project is named INDXParse. A script installed by the packing setup is named INDXParse.py. That script also provides some functionality as a module, exporting symbols re-used in other scripts used in this package.

The package, before this patch, was called INDXParse. Re-use of the name spelled as INDXParse unfortunately caused some runtime issues when attempting to call package-installed scripts from the command line. The last line of the stack trace from calling MFTINDX.py reported this:

ModuleNotFoundError: No module named 'INDXParse.MFTINDX'; 'INDXParse' is not a package

A StackOverflow thread (see refs) showed one resolution to this issue was de-conflicting the name of the package from the name of a module in the package. In the StackOverflow thread, adding _base to the included module resolved the poster's issue.

PEP-8 includes a prescription that goes part of the way to resolving the package/module name conflict: Package names are suggested to be lowercase, rather than the mixed-case used by the INDXParse project. This patch applies that convention to the package name.

One matter left unresolved is that PEP-8 also suggests module names be lowercase. If the INDXParse.py file is renamed to indxparse.py, the same ModuleNotFoundError described above is likely to recur.

Since Issue 40 is considering splitting modules from scripts, the question of how to rename INDXParse.py is left for a future patch. This patch is scoped to addressing the runtime issue.

This patch was tested by installing this package into a virtual environment and running MFTINDX.py against a test disk image. That call failed, as the script's full functionality hasn't yet been restored since the Python 2 code state; but it did run to a more interesting internal code point, rather than fail immediately with a ModuleNotFoundError.

References:

This patch addresses a runtime issue not found in PR 46.

This project is named INDXParse.  A script installed by the packing
setup is named INDXParse.py.  That script also provides some
functionality as a module, exporting symbols re-used in other scripts
used in this package.

The package, before this patch, was called `INDXParse`.  Re-use of the
name spelled as `INDXParse` unfortunately caused some runtime issues
when attempting to call package-installed scripts from the command line.
The last line of the stack trace from calling `MFTINDX.py` reported this:

    ModuleNotFoundError: No module named 'INDXParse.MFTINDX'; 'INDXParse' is not a package

A StackOverflow thread (see refs) showed one resolution to this issue
was de-conflicting the name of the package from the name of a module in
the package.  In the StackOverflow thread, adding `_base` to the
included module resolved the poster's issue.

PEP-8 includes a prescription that goes part of the way to resolving the
package/module name conflict: Package names are suggested to be
lowercase, rather than the mixed-case used by the INDXParse project.
This patch applies that convention to **the package name**.

One matter left unresolved is that PEP-8 also suggests module names be
lowercase.  If the INDXParse.py file is renamed to indxparse.py, the
same `ModuleNotFoundError` described above is likely to recur.

Since Issue 40 is considering splitting modules from scripts, the
question of how to rename INDXParse.py is left for a future patch.  This
patch is scoped to addressing the runtime issue.

This patch was tested by installing this package into a virtual
environment and running MFTINDX.py against a test disk image.  That call
failed, as the script's full functionality hasn't yet been restored
since the Python 2 code state; but it did run to a more interesting
internal code point, rather than fail immediately with a
`ModuleNotFoundError`.

References:
* williballenthin#40
* williballenthin#46
* https://peps.python.org/pep-0008/#package-and-module-names
* https://stackoverflow.com/a/59362330

Signed-off-by: Alex Nelson <[email protected]>
@ajnelson-nist ajnelson-nist mentioned this pull request Jul 25, 2023
Copy link
Owner

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the detailed explanation. makes sense to me and the changes look good.

@ajnelson-nist
Copy link
Contributor Author

I'm glad they look good! I just pushed a catch-up merge to get past the conflict GitHub was reporting.

@williballenthin williballenthin merged commit 8ab39de into williballenthin:master Jul 25, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants