Migrating to PathLib instead of os.path

[huguesdevimeux]

The title. A lot of stuff are still using os.path to handle paths, when Pathlib seems to be more appropriate as more recent (and built-in, of course). In particular, pretty much everything within SceneFileWriter uses os.path.

Very easy PR ;)

[naveen521kk]

(mark it as Hacktoberfest)

[raisultan]

hi👋 @huguesdevimeux, I would love to help!

[naveen521kk]

Sure @raisultan I will assign you.

[raisultan]

@huguesdevimeux, thanks

[leotrs]

@raisultan @digitalvirtuoso what's the status on this?

[digitalvirtuoso]

@leotrs There were still modules that needed os to pathlib. I've been pretty busy lately. I think it was a good time to merge my PR :)

[leotrs]

Thanks for the answer! I'll keep this open then.

[raisultan]

@leotrs, hi! Sorry, I was a bit busy lately, gonna make a PR on the weekend.

[behackl]

Looks like there are still plenty of os.path left in the library. Marking this as a good first issue. (Oh well, it already was labeled as that. Sorry for the noise.)

[IlievskiV]

Hi, is there anyone working on this issue? If not, I can take it over :)

[behackl]

Hi, is there anyone working on this issue? If not, I can take it over :)

Please go ahead! I can't recall how many occurrences of os.path really were left; feel free to submit partial work as well. :-)

[SaiSridhar783]

is this issue still open? if yes, can you assign me?

[RoshanSalian]

I downloaded the repository and ran 'poetry run pytest' on it. This is done after 'git pull upstream main'. However even after this lot of testcase are failing. Is it normal behavior?

=========================== short test summary info =========================== FAILED tests/test_coordinate_system.py::test_NumberPlane - RuntimeError: late... FAILED tests/test_copy.py::test_bracelabel_copy - RuntimeError: latex failed ... FAILED tests/test_number_line.py::test_add_labels - RuntimeError: latex faile... FAILED tests/test_numbers.py::test_font_size - RuntimeError: latex failed but... FAILED tests/test_numbers.py::test_font_size_vs_scale - RuntimeError: latex f... FAILED tests/test_numbers.py::test_changing_font_size - RuntimeError: latex f... FAILED tests/test_numbers.py::test_set_value_size - RuntimeError: latex faile... FAILED tests/test_texmobject.py::test_MathTex - RuntimeError: latex failed bu... FAILED tests/test_texmobject.py::test_SingleStringMathTex - RuntimeError: lat... FAILED tests/test_texmobject.py::test_double_braces_testing[{{ a }} + {{ b }} = {{ c }}-5] FAILED tests/test_texmobject.py::test_double_braces_testing[\\frac{1}{a+b\\sqrt{2}}-1] FAILED tests/test_texmobject.py::test_tex - RuntimeError: latex failed but di... FAILED tests/test_texmobject.py::test_tex_whitespace_arg - RuntimeError: late... FAILED tests/test_texmobject.py::test_tex_non_whitespace_arg - RuntimeError: ... FAILED tests/test_texmobject.py::test_tex_white_space_and_non_whitespace_args FAILED tests/test_texmobject.py::test_tex_size - RuntimeError: latex failed b... FAILED tests/test_texmobject.py::test_font_size - RuntimeError: latex failed ... FAILED tests/test_texmobject.py::test_font_size_vs_scale - RuntimeError: late... FAILED tests/test_texmobject.py::test_changing_font_size - RuntimeError: late... FAILED tests/opengl/test_copy_opengl.py::test_bracelabel_copy - RuntimeError:... FAILED tests/opengl/test_number_line_opengl.py::test_add_labels - RuntimeErro... FAILED tests/opengl/test_numbers_opengl.py::test_font_size - RuntimeError: la... FAILED tests/opengl/test_numbers_opengl.py::test_font_size_vs_scale - Runtime... FAILED tests/opengl/test_numbers_opengl.py::test_changing_font_size - Runtime... FAILED tests/opengl/test_numbers_opengl.py::test_set_value_size - RuntimeErro... FAILED tests/opengl/test_texmobject_opengl.py::test_MathTex - RuntimeError: l... FAILED tests/opengl/test_texmobject_opengl.py::test_SingleStringMathTex - Run... FAILED tests/opengl/test_texmobject_opengl.py::test_double_braces_testing[{{ a }} + {{ b }} = {{ c }}-5] FAILED tests/opengl/test_texmobject_opengl.py::test_double_braces_testing[\\frac{1}{a+b\\sqrt{2}}-1] FAILED tests/opengl/test_texmobject_opengl.py::test_tex - RuntimeError: latex... FAILED tests/opengl/test_texmobject_opengl.py::test_tex_whitespace_arg - Runt... FAILED tests/opengl/test_texmobject_opengl.py::test_tex_non_whitespace_arg - ... FAILED tests/opengl/test_texmobject_opengl.py::test_tex_white_space_and_non_whitespace_args FAILED tests/opengl/test_texmobject_opengl.py::test_tex_size - RuntimeError: ... FAILED tests/opengl/test_texmobject_opengl.py::test_font_size - RuntimeError:... FAILED tests/opengl/test_texmobject_opengl.py::test_font_size_vs_scale - Runt... FAILED tests/opengl/test_texmobject_opengl.py::test_changing_font_size - Runt... FAILED tests/test_graphical_units/test_axes.py::test_axes - RuntimeError: lat... FAILED tests/test_graphical_units/test_axes.py::test_custom_coordinates - Run... FAILED tests/test_graphical_units/test_axes.py::test_get_axis_labels - Runtim... FAILED tests/test_graphical_units/test_axes.py::test_get_x_axis_label - Runti... FAILED tests/test_graphical_units/test_axes.py::test_get_y_axis_label - Runti... FAILED tests/test_graphical_units/test_axes.py::test_get_graph_label - Runtim... FAILED tests/test_graphical_units/test_axes.py::test_get_line_graph - Runtime... FAILED tests/test_graphical_units/test_axes.py::test_t_label - RuntimeError: ... FAILED tests/test_graphical_units/test_axes.py::test_get_area - RuntimeError:... FAILED tests/test_graphical_units/test_axes.py::test_get_z_axis_label - Runti... FAILED tests/test_graphical_units/test_coordinate_systems.py::test_number_plane FAILED tests/test_graphical_units/test_numbers.py::test_set_value_with_updaters FAILED tests/test_graphical_units/test_opengl.py::test_Circle - AssertionError: FAILED tests/test_graphical_units/test_tables.py::test_Table - RuntimeError: ... FAILED tests/test_graphical_units/test_tables.py::test_MathTable - RuntimeErr... FAILED tests/test_graphical_units/test_tables.py::test_IntegerTable - Runtime... FAILED tests/test_graphical_units/test_tables.py::test_DecimalTable - Runtime... FAILED tests/test_graphical_units/test_vector_scene.py::test_vector_to_coords FAILED tests/test_scene_rendering/test_caching_related.py::test_play_skip - A... FAILED tests/test_scene_rendering/test_cairo_renderer.py::test_skipping_status_with_from_to_and_up_to FAILED tests/test_scene_rendering/test_play_logic.py::test_t_values_with_cached_data FAILED tests/test_scene_rendering/opengl/test_caching_related_opengl.py::test_play_skip FAILED tests/test_scene_rendering/opengl/test_cli_flags_opengl.py::test_n_flag FAILED tests/test_scene_rendering/test_cli_flags.py::test_n_flag - AssertionE... = 61 failed, 453 passed, 2 skipped, 8 xfailed, 1 xpassed, 227 warnings in 254.08s (0:04:14) =

[behackl]

I downloaded the repository and ran 'poetry run pytest' on it. This is done after 'git pull upstream main'. However even after this lot of testcase are failing. Is it normal behavior?

Not sure this is related to this particular issue. Some of the tests seem to fail because there is a problem with latex, do you have a working LaTeX distribution on your system? It might be worth opening a separate issue for this; locally all tests pass on my system.

[basil08]

@behackl I would like to look into this. Can you assign me?

[behackl]

@behackl I would like to look into this. Can you assign me?

No need for assignments; feel free to go ahead and submit a PR. The same comment as in https://github.com/ManimCommunity/manim/issues/485#issuecomment-932518829 holds.

[jbalpert]

Hey, I noticed that there are still elements of os.path still needing to be fixed! I would love to convert those to pathlib unless someone is currently working on it.

[Darylgolden]

Please go ahead!

[hickmott99]

I will assist @jbalpert!

[jbalpert]

Update on the Pathlib issue @hickmott99 @Darylgolden we currently have modified the following files, we will be working on it later today too!

        modified:   manim/cli/cfg/group.py
        modified:   manim/mobject/svg/svg_mobject.py
        modified:   manim/mobject/text/text_mobject.py
        modified:   manim/utils/tex_file_writing.py

[kitrakrev]

Can I help with this? if yes, which are the files I am supposed to edit?

[detective-sokka]

I would love to work on this. @kitrak-rev @jbalpert.

[behackl]

Hey @detective-sokka, I believe that this issue is actually resolved by now. Checking for current appearances of os.path in the repository reveals

$ grep -r "os\.path" manim/ tests/
manim/scene/scene_file_writer.py:                key=os.path.getatime,
manim/scene/scene_file_writer.py:            # oldest_file_path = min(cached_partial_movies, key=os.path.getatime)
manim/utils/file_ops.py:    os.utime(file_path, times=(time.time(), os.path.getmtime(file_path)))

Feel free to check whether / how os.path can be replaced by pathlib in these two remaining instances and submit a PR -- but the main migration has already gradually happened via #2980 and #2991.

Perhaps you feel like checking out the current state of #2959 to help getting it merged? Feel free to hop on Discord to discuss other issues which need help.

[detective-sokka]

Thank you for the reply @behackl. I'll check #2959 and the discord