endangeredoxen / fivecentplots

A Python plotting analgesic
https://endangeredoxen.github.io/fivecentplots
11 stars 6 forks source link

Closes #19 #20

Closed FaustinCarter closed 4 years ago

FaustinCarter commented 4 years ago

I figure if I'm going to complain the least I can do is help with a PR!

Closes #19

After doing a little treasure hunt of import errors I've updated the package requirements to reflect the full list of packages required for fivecentplots to import after install into an empty envioronment.

I didn't touch the conda package file generator script. It's worth mentioning that "opencv-python" in the pypi repo is "opencv" in the anaconda repo.

I ran tests on this install, and the output is below:

(fcp) λ pytest fivecentplots
================================================= test session starts =================================================
platform win32 -- Python 3.8.3, pytest-5.4.3, py-1.8.2, pluggy-0.13.1
rootdir: C:\Users\fwcarter\Desktop\code\fivecentplots
collected 164 items

fivecentplots\tests\test_barplot.py .......                                                                      [  4%]
fivecentplots\tests\test_boxplot.py ....................                                                         [ 16%]
fivecentplots\tests\test_contour.py FF.                                                                          [ 18%]
fivecentplots\tests\test_grouping.py FF....F.....FFFFFFF.                                                        [ 30%]
fivecentplots\tests\test_heatmap.py .......                                                                      [ 34%]
fivecentplots\tests\test_hist.py .......                                                                         [ 39%]
fivecentplots\tests\test_misc.py FFFFFF                                                                          [ 42%]
fivecentplots\tests\test_plot.py .FF.F..FFFFFFF....FF....FFF                                                     [ 59%]
fivecentplots\tests\test_ranges.py FFFF.......FFFFFFFF                                                           [ 70%]
fivecentplots\tests\test_styles.py FFFFFFFFFFFFFFFFF...FFFFFFF                                                   [ 87%]
fivecentplots\tests\test_ticks.py .............F.......                                                          [100%]

====================================================== FAILURES =======================================================

Skipping lots and lots of stack traces...

================================================== warnings summary ===================================================
fivecentplots/tests/test_hist.py: 25 tests with warnings
fivecentplots/tests/test_styles.py: 4 tests with warnings
  Warning: Passing normed=False is deprecated, and has no effect. Consider passing the density argument instead.

fivecentplots/tests/test_hist.py::test_kde
  Warning: Passing `normed=True` on non-uniform bins has always been broken, and computes neither the probability density function nor the probability mass function. The result is only correct if the bins are uniform, when density=True will produce the same result anyway. The argument will be removed in a future version of numpy.

-- Docs: https://docs.pytest.org/en/latest/warnings.html
=============================================== short test summary info ===============================================
FAILED fivecentplots/tests/test_contour.py::test_basic - assert not True
FAILED fivecentplots/tests/test_contour.py::test_filled - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_legend_single - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_legend_multiple - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_legend_position - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_groups_row_col - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_groups_row_col_y - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_groups_row_col_x - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_groups_wrap_unique - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_groups_wrap_column_ncol - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_groups_wrap_xy - assert not True
FAILED fivecentplots/tests/test_grouping.py::test_groups_wrap_names_no_sharing - assert not True
FAILED fivecentplots/tests/test_misc.py::test_text_box_single - assert not True
FAILED fivecentplots/tests/test_misc.py::test_text_box_single_style - assert not True
FAILED fivecentplots/tests/test_misc.py::test_text_box_multiple - assert not True
FAILED fivecentplots/tests/test_misc.py::test_text_box_position_figure - assert not True
FAILED fivecentplots/tests/test_misc.py::test_text_box_position_data - assert not True
FAILED fivecentplots/tests/test_misc.py::test_text_box_position_units - assert not True
FAILED fivecentplots/tests/test_plot.py::test_xy_legend - assert not True
FAILED fivecentplots/tests/test_plot.py::test_xy_log_scale - assert not True
FAILED fivecentplots/tests/test_plot.py::test_xy_ts - assert not True
FAILED fivecentplots/tests/test_plot.py::test_multiple_xy_y - assert not True
FAILED fivecentplots/tests/test_plot.py::test_multiple_xy_x - assert not True
FAILED fivecentplots/tests/test_plot.py::test_multiple_xy_both - assert not True
FAILED fivecentplots/tests/test_plot.py::test_row - assert not True
FAILED fivecentplots/tests/test_plot.py::test_column - assert not True
FAILED fivecentplots/tests/test_plot.py::test_row_x_column - assert not True
FAILED fivecentplots/tests/test_plot.py::test_wrap - assert not True
FAILED fivecentplots/tests/test_plot.py::test_other_curve_fitting_legend - assert not True
FAILED fivecentplots/tests/test_plot.py::test_other_curve_fitting_legend2 - assert not True
FAILED fivecentplots/tests/test_plot.py::test_other_ref_line - assert not True
FAILED fivecentplots/tests/test_plot.py::test_other_ref_line_mult - assert not True
FAILED fivecentplots/tests/test_plot.py::test_other_ref_line_complex - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_default - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_primary - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_primary_no_scale - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_primary_explicit - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_boxplot_quantile - TypeError: 'quantile' cannot be performed against ...
FAILED fivecentplots/tests/test_ranges.py::test_boxplot_iqr - TypeError: 'quantile' cannot be performed against 'obje...
FAILED fivecentplots/tests/test_ranges.py::test_shared - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_shared_false - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_shared_separate - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_shared_rows - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_shared_cols - assert not True
FAILED fivecentplots/tests/test_ranges.py::test_shared_no - assert not True
FAILED fivecentplots/tests/test_styles.py::test_fill_color - assert not True
FAILED fivecentplots/tests/test_styles.py::test_edge_color - assert not True
FAILED fivecentplots/tests/test_styles.py::test_spines - assert not True
FAILED fivecentplots/tests/test_styles.py::test_alpha - assert not True
FAILED fivecentplots/tests/test_styles.py::test_alpha_marker - assert not True
FAILED fivecentplots/tests/test_styles.py::test_alpha_legend_marker - assert not True
FAILED fivecentplots/tests/test_styles.py::test_line_color - assert not True
FAILED fivecentplots/tests/test_styles.py::test_line_color_custom - assert not True
FAILED fivecentplots/tests/test_styles.py::test_line_color_index - assert not True
FAILED fivecentplots/tests/test_styles.py::test_line_color_cmap - assert not True
FAILED fivecentplots/tests/test_styles.py::test_line_style - assert not True
FAILED fivecentplots/tests/test_styles.py::test_line_style2 - assert not True
FAILED fivecentplots/tests/test_styles.py::test_line_style_by_line - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_edge - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_fill - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_fill_default - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_fill_alpha - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_type - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_type_none - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_size - assert not True
FAILED fivecentplots/tests/test_styles.py::test_marker_size_legend - assert not True
FAILED fivecentplots/tests/test_styles.py::test_fonts - assert not True
FAILED fivecentplots/tests/test_styles.py::test_theme_white - assert not True
FAILED fivecentplots/tests/test_styles.py::test_theme_gray - assert not True
FAILED fivecentplots/tests/test_ticks.py::test_tick_cleanup_off - assert not True
=============================== 70 failed, 94 passed, 30 warnings in 135.62s (0:02:15) ================================
endangeredoxen commented 4 years ago

Thanks for jumping on this!

Most likely the tests are failing because of differences between the version of mpl I used to generate the test images and the version you are using. Generally speaking, the tests are not part of the package download so I don't want to require dependencies that only are used for developers (opencv falls into this category).

I'm also considering making mpl the only required plotting library and adding exceptions/warnings for other engines like bokeh to keep the install minimal. pillow is also only used with bokeh.

FaustinCarter commented 4 years ago

No worries. Glad to help contribute to something that is useful. FWIW, I've run into a lot of similar issues with trying to test a another package (pygtc) based on comparing images. I've found that in general one can't compare images generated with different OSs and also one can't compare images generated with different versions of freetype. There's a discussion of this here: https://github.com/SebastianBocquet/pygtc/pull/21. In the end I think we just generated a set of test images for OSX and another one for Windows.

endangeredoxen commented 4 years ago

Great point on the platform differences. Your suggestion makes sense

On Sat, Jun 27, 2020, 8:20 PM Faustin Carter notifications@github.com wrote:

No worries. Glad to help contribute to something that is useful. FWIW, I've run into a lot of similar issues with trying to test a another package (pygtc) based on comparing images. I've found that in general one can't compare images generated with different OSs and also one can't compare images generated with different versions of freetype. There's a discussion of this here: SebastianBocquet/pygtc#21 https://github.com/SebastianBocquet/pygtc/pull/21. In the end I think we just generated a set of test images for OSX and another one for Windows.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/endangeredoxen/fivecentplots/pull/20#issuecomment-650673310, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADMCOIL74N67P4M7N2YFKCLRY2SGRANCNFSM4OAYXASQ .

endangeredoxen commented 4 years ago

I decided to make only mpl required which changed some of the required installs. I added exceptions as needed. A clean build works on my end now.

pip install fivecentplots

Let me know if you have any issues and thanks for raising the issue

FaustinCarter commented 4 years ago

Hey, sorry for the delay in responding; I've been in the middle of moving for the last two weeks, but finally settled. Anyhow, I installed in a clean environment, and importing worked fine. I didn't test anything else, but it looks like it's been resolved.

I would recommend keeping this line in setup.py:

    extras_require={
                    'test': ['pytest', 'opencv-python', 'imageio']
    },

Or whatever the equivalent is. It will make it easy for folks who want to install a test build to do so, by just doing pip install fivecentplots[test]. It also means that any devs who want to muck around can immediately see from setup.py that tests will require additional deps, rather than finding out when they get the warning message. Or, you could do something like:

    extras_require={
                    'test': ['pytest'],
                    'image_test': ['pytest', 'opencv-python', 'imageio']
    },

and give some options.

Just my $.02. I'm going to close this PR since it looks like your code is good to roll.

Cheers!

endangeredoxen commented 4 years ago

No worries. Good tip on extras_require. Didn't know that and will add it as you suggested. Best of luck post move

On Sat, Jul 11, 2020 at 3:34 PM Faustin Carter notifications@github.com wrote:

Hey, sorry for the delay in responding; I've been in the middle of moving for the last two weeks, but finally settled. Anyhow, I installed in a clean environment, and importing worked fine. I didn't test anything else, but it looks like it's been resolved.

I would recommend keeping this line in setup.py:

extras_require={
                'test': ['pytest', 'opencv-python', 'imageio']
},

Or whatever the equivalent is. It will make it easy for folks who want to install a test build to do so, by just doing pip install fivecentplots[test]. It also means that any devs who want to muck around can immediately see from setup.py that tests will require additional deps, rather than finding out when they get the warning message. Or, you could do something like:

extras_require={
                'test': ['pytest'],
                'image_test': ['pytest', 'opencv-python', 'imageio']
},

and give some options.

Just my $.02. I'm going to close this PR since it looks like your code is good to roll.

Cheers!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/endangeredoxen/fivecentplots/pull/20#issuecomment-657133699, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADMCOILDIKPKPLL27WSZSLDR3DLH7ANCNFSM4OAYXASQ .