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

Added a more advanced method to locate the maindir from the Python scripts #212

Merged
merged 2 commits into from
Apr 16, 2024

Conversation

hakonhagland
Copy link
Collaborator

For background see #209.

Added a more advanced method to locate the maindir and filename arguments for usage with the Python scripts. This is useful because the Python scripts can be installed globally and then run from any directory. The original logic assumed that the Python scripts were run from scripts/python. Also added some unit tests.

Added a more advanced method to locate maindir and filename for usage
with the python scripts.
@bska
Copy link
Member

bska commented Apr 12, 2024

It may or may not be useful–I don't know the full context here–but the main git(1) command line utility has

git rev-parse --git-dir

which solves this exact problem.

@hakonhagland
Copy link
Collaborator Author

@bska Thanks for the info. I think I'll keep the current solution for now though. As it will not require calling an external program (git) from the Python script. However, it is very useful to know that git has this location-of-root-dir stuff builtin 😄

@bska
Copy link
Member

bska commented Apr 12, 2024

it is very useful to know that git has this location-of-root-dir stuff builtin

It is indeed. That command also knows about the GIT_DIR environment variable for the (exceedingly rare) case that someone actually has a non-default GIT_DIR.

@lisajulia
Copy link
Collaborator

lisajulia commented Apr 16, 2024

I'm getting this error here - did I do something wrong or is this really a bug introduced by this PR?

Traceback (most recent call last):
  File "/home/lnebel/python-venv/bin/fodt-remove-span-tags", line 6, in <module>
    sys.exit(remove_version_span_tags())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lnebel/python-venv/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lnebel/python-venv/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/lnebel/python-venv/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lnebel/python-venv/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lnebel/git_repos/opm/opm-reference-manual/scripts/python/src/fodt/remove_span_tags.py", line 350, in remove_version_span_tags
    maindir, filename = Helpers.locate_maindir_and_filename(maindir, filename)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lnebel/git_repos/opm/opm-reference-manual/scripts/python/src/fodt/helpers.py", line 108, in locate_maindir_and_filename
    filename = Path(filename)
               ^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/pathlib.py", line 872, in __new__
    self = cls._from_parts(args)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/pathlib.py", line 510, in _from_parts
    drv, root, parts = self._parse_args(args)
                       ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/pathlib.py", line 494, in _parse_args
    a = os.fspath(a)
        ^^^^^^^^^^^^
TypeError: expected str, bytes or os.PathLike object, not NoneType

@hakonhagland
Copy link
Collaborator Author

@lisajulia Thanks for the report. Can you show the exact command line you used to produce the backtrace?

@lisajulia
Copy link
Collaborator

lisajulia commented Apr 16, 2024

@lisajulia Thanks for the report. Can you show the exact command line you used to produce the backtrace?

Sorry, yes, here it is:

(python-venv) lnebel:~/git_repos/opm/opm-reference-manual/scripts/python$ fodt-remove-span-tags 

I chose the command fodt-remove-span-tags because the new method to locate the maindir is used in scripts/python/src/fodt/remove_span_tags.py

Improved the algorithm for location of maindir when filename is not
given. Fixed remove-span-tags script to not assume that filename is
relative to maindir
@hakonhagland
Copy link
Collaborator Author

@lisajulia Seems like the script assumed that filename would be relative to the maindir. I have improved the algorithm in the latest commit also added some more test cases.

@lisajulia lisajulia merged commit 81682fd into OPM:main Apr 16, 2024
6 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.

None yet

3 participants