Fix failing tests

[huguesdevimeux]

There are three issues that I fixed :

1 : pytest rootdir

The first issue was a quick one, as the rootdir of pytest was set as manim/, the .npy files could not be found as their path was set to tests_data/etc and not tests/tests_data/etc.
I think that this was an inteniontal move by @leotrs, who wanted the tests to use a custom manim.cfg. Sadly, we can't currently use one when not rendering a scene not from command lines. See #184.

2 : skip_animation not enabled for tests : a t values issue.

The second issue the these failing tests undercovered was an issue with t values.
@leotrs changed a little bit the config that the tests use when they are running, he basically didn't add -s flag. I agree, this should not be a problem as the last frame should be the same if we skip the rendering process of the animations or not, but they are different. See #183 TLDR : This is a problem with the t values, when doing -s, there is only one t value generated by manim wich is 1.0 (the time when the animation ends), and when the skip_animations is False, there are, let's say, 15 values of t generated (as 15 fps) which go from 0 to not 1 but .93333. In other terms, the animation stops at 93%). I fixed THE TESTS (not this issue) by simply adding to testing_utils.py file_write_config['skip_animations'] = True.

3 : frame_width slightly changed

I didn't have to fix this, I will explain.
frame_width is computed l62 config.py :

    config['frame_width'] = (config['frame_height']
                             * config['pixel_width']
                             / config['pixel_height'])

Because of how Python handles float, this value may differ depending how what is the value of config['pixel_width'] and config['pixel_height']
If config['pixel_width'] is not set in a manim.cfg, (as it is done in the tests, see testing_utils.py, l.48), default value of pixel_width will be taken.
There is the problem, config['frame_width'] should be the same in both cases (set in manim.cdg and not set) (as default values and values used in the tests have the same ratio), they slightly differ (something like 14.211111111111 and 14.23333333).
That's why there is a slightly pixel related difference.
As we don't use a manim.cfg for the tests rn, it is ok. But when we will do so, we will have to render a new set of tests_data.

Issues to be fixed :

• NameError: name 'FRAME_X_RADIUS' is not defined
  Not sure how to fix that. The king @leotrs surely knows.
  FIXED

• UnicodeEncodeError: 'charmap' codec can't encode character '\u2764' in position 2905: character maps to <undefined>
  This has been recently added in the -h flag : "Made with ❤ by the manim community dev".
  Sadly, '\u2764' (the heart) is seems to not be suported by the decoding of subprocess.
  To quicly fix this major issue, we could do the biggest change in manim's history : changing ❤ by <3.
  FIXED

[leotrs]

1. I think I meant to reverse this change at some point but failed to. My apologies!

2. During the development of Configparser setup: figuring out config.py, constants.py, dirs.py, CLI arguments, and more [WIP] #98, skip_animations was definitely a sticking point. There is some logic in config.py to set it, but it's not as general as it should be. For example, it seems it's not being set correctly when ran from the testing suite. The main problem with this is that skip_animations is fixed by the values of other config options, and not by a config option itself (that is, it cannot be set with its own config option from default.cfg currently). I'm all ears for suggestions here.

3. Excellent catch here! What was the fix?

Standing issues.

• Change every occurrence of FRAME_X_RADIUS to config['frame_x_radius']. Make sure to from manim.config import config

• It originally said <3, and then someone changed it to the heart emoji/unicode. I will ping @eulertour here for no particular reason whatsoever ;)

[huguesdevimeux]

2. Yes, I saw that. I would suggest adding skip_animation to the ArgumentParser thing and doing something like this :

    config['skip_animations'] = any([config['save_last_frame'],
                                     config['from_animation_number'],
                                     default.get_boolean('skip_animations')])

I don't know if it makes sense, but it could work.

3. I don't think there is a fix lol. The solution would be to re-render the pre-rendered frames.

[leotrs]

2. I hesitate to keep adding more CLI arguments, especially since the point is that there is already some logic for the option otherwise. Unless we add a new option that just overrides that default logic. (Man this friggin config system is so convoluted..)

3. No but I mean, we shouldn't have fractional frame widths anywhere, right? We have to round somewhere.

[huguesdevimeux]

2. You're right, skip_animation does not need to be a CLI argument, but need IMO to be available in manim.cfg We can do this and keep the logic, I think.

1. Man this friggin config system is so convoluted..)

Arh, mathematicians, always complicated ..

2. I see you're point, but tbh I don't have an opinion on that. I think rounding could cause problems when one wants a precision, but I don't know enough about that to really say something constructive lol.
   I don't even know why is frame_height set to 8 by default, as it is modified after in camera.py ..
   Seems to be a little messy over there :/

[leotrs]

1. Ok, let's do that. skip_animations wouldn't be the only config option that is on default.cfg but not available through CLI flags.

2. That's fair. Let's open another issue on this, since it's separate from "fix failing tests". Also, rounding that may lead to unexpected bugs, as you say.

[eulertour]

Add encoding='utf-8' here to fix the encoding issue.

manim/tests/test_cli/test_cli.py

Lines 8 to 11 in 7a088cf

	proc = subprocess.Popen(command,
	stdout=subprocess.PIPE,
	stderr=subprocess.PIPE,
	)

It's a bit worrying that 1/14th of 2/100ths of a difference, which appears to within the margin of error of float operations in python, would cause a test failure. Still, can see if the tests are robust enough to handle modifications later on.

[leotrs]

I think we have tests again ;)