OPM / opm-reference-manual

Other
1 stars 5 forks source link

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

Closed hakonhagland closed 5 months ago

hakonhagland commented 6 months ago

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.

bska commented 5 months ago

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 commented 5 months ago

@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 :smile:

bska commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

lisajulia commented 5 months ago

@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

hakonhagland commented 5 months ago

@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.