-
Notifications
You must be signed in to change notification settings - Fork 228
Finalize fixes on WIndows test suite for v0.1.1 #441
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
Conversation
Exact error message was "ValueError: path is on mount 'C:', start on mount 'D:'."
Last 2 tests to fix! Investigating now, might be related to how ================================== FAILURES ===================================
__________________ [doctest] pygmt.clib.conversion._as_array __________________
221
222 Examples
223 --------
224
225 >>> import pandas as pd
226 >>> x_series = pd.Series(data=[1, 2, 3, 4])
227 >>> x_array = _as_array(x_series)
228 >>> type(x_array)
229 <class 'numpy.ndarray'>
230 >>> x_array
Expected:
array([1, 2, 3, 4])
Got:
array([1, 2, 3, 4], dtype=int64)
C:\Miniconda\envs\testing\Lib\site-packages\pygmt\clib\conversion.py:230: DocTestFailure
___________ [doctest] pygmt.clib.conversion.kwargs_to_ctypes_array ____________
263 Returns
264 -------
265 ctypes_value : ctypes array or None
266
267 Examples
268 --------
269
270 >>> import ctypes as ct
271 >>> value = kwargs_to_ctypes_array('bla', {'bla': [10, 10]}, ct.c_int*2)
272 >>> type(value)
Expected:
<class 'pygmt.clib.conversion.c_int_Array_2'>
Got:
<class 'pygmt.clib.conversion.c_long_Array_2'>
C:\Miniconda\envs\testing\Lib\site-packages\pygmt\clib\conversion.py:272: DocTestFailure |
Perhaps change the tests a little bit. For example, changing
to
|
The .values method isn't recommended anymore for pandas 1.0, see https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Series.values.html#pandas.Series.values. Using to_numpy() instead.
Because running np.asarray on a pandas.Series returns a numpy.ndarray anyway.
According to https://docs.python.org/3/library/ctypes.html#ctypes-tutorial, `c_int` is just an alias for `c_long`.
>>> value = kwargs_to_ctypes_array('bla', {'bla': [10, 10]}, ct.c_int*2) | ||
>>> value = kwargs_to_ctypes_array('bla', {'bla': [10, 10]}, ct.c_long*2) | ||
>>> type(value) | ||
<class 'pygmt.clib.conversion.c_int_Array_2'> | ||
<class 'pygmt.clib.conversion.c_long_Array_2'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c_int
is just an alias for c_long
according to https://docs.python.org/3/library/ctypes.html#ctypes-tutorial. Changing it to c_long
should make this test pass in a cross-platform way.
Awesome, all green and good to go! |
Description of proposed changes
Towards a fully working Windows test suite! Several of our unit tests (~8) have been failing since #434, and we've addressed them in several PRs already. This one will hopefully squash the last of them.
Merge after these are completed (in order):
logo
image discrepancy with conda ghostscript (Let PyGMT work with the conda GMT package on Windows #434)gmt.dll
withoutGMT_LIBRARY_PATH
environment variable (Improve how PyGMT finds the GMT library #440)Fixes #
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.