Skip to content
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

Updating GitHub actions #4860

Merged
merged 19 commits into from
Apr 13, 2021
Merged

Updating GitHub actions #4860

merged 19 commits into from
Apr 13, 2021

Conversation

hoxbro
Copy link
Member

@hoxbro hoxbro commented Mar 8, 2021

I have been working on updating the test GitHub Actions and I have come to a point where I want to share it. It is not necessary that this PR is to be merged, but the changes I have made could be added to a future PR.

Overview over the changes I have done

  • Only accept Python 3.6, 3.7, and 3.8. Maybe this is a bit harsh, but as I understand the 1.14 release will be the last to support 2.7.
  • Added conda-forge to channels. This is maybe not needed anymore.
  • Loosen some pins, which does not seem to be needed anymore.
  • Updated matplotlib/datetime related unittest to be compatible with matplotlib 3.3, the changes is because matplotlib have changed their date2num from a custom format to UNIX time, see example code below. Maybe matplotlib should be pinned for this?
  • Changed values of 2 others related to matplotlib update. I would like to know if this is ok?
  • I added mamba to replace conda install but removed it again. I think it could be a good option to add to pyct/pyctdev.

Example code of change to matplotlib

# located at https://matplotlib.org/3.3.0/_modules/matplotlib/dates.html#_from_ordinalf
# changed the functions a bit to be more compact.

SEC_PER_DAY = 60 * 60 * 24

def new_dt64_to_ordinalf(d):
    # From matplotlib version 3.3.0
    dseconds = d.astype('datetime64[s]')
    extra = (d - dseconds).astype('timedelta64[ns]')
    t0 = np.datetime64("1970-01-01T00:00:00", 's')
    dt = (dseconds - t0).astype(np.float64)
    dt += extra.astype(np.float64) / 1.0e9
    dt = dt / SEC_PER_DAY
    return dt

def old_dt64_to_ordinalf(d):
    # From matplotlib version 3.0.3
    extra = d - d.astype('datetime64[s]').astype(d.dtype)
    extra = extra.astype('timedelta64[ns]')
    t0 = np.datetime64('0001-01-01T00:00:00').astype('datetime64[s]')
    dt = (d.astype('datetime64[s]') - t0).astype(np.float64)
    dt += extra.astype(np.float64) / 1.0e9
    dt = dt / SEC_PER_DAY + 1.0
    return dt

x = np.datetime64('2016-04-04', 'ns')
old_dt64_to_ordinalf(x), new_dt64_to_ordinalf(x), old_dt64_to_ordinalf(x) - new_dt64_to_ordinalf(x)
# (736058.0, 16895.0, 719163.0)

@jlstevens
Copy link
Contributor

Thanks for the PR!

Only accept Python 3.6, 3.7, and 3.8. Maybe this is a bit harsh, but as I understand the 1.14 release will be the last to support 2.7.

That is correct though we expect at least one more release in the 1.14 series.

Added conda-forge to channels. This is maybe not needed anymore.

Generally we avoid using conda-forge unless necessary as including it slows down solve times.

Thanks for the matplotlib 3.3 compatibility updates!

@philippjfr
Copy link
Member

Thanks @hoxbro! I had deliberately not updated the matplotlib tests so I could keep them passing on py2 and py3 builds. Having dropped the py2 builds updating them is the correct thing to do.

@philippjfr
Copy link
Member

This needs rebasing but I'm quite happy to now make the master branch drop Python 2 support and backport commits to a 1.14.x branch if necessary.

@hoxbro hoxbro force-pushed the mac_conda_develop branch from c16fda6 to 18fc4a6 Compare April 10, 2021 15:33
@hoxbro
Copy link
Member Author

hoxbro commented Apr 10, 2021

I have made a rebase of the PR.

If this PR is being merged I need to do some small clean ups and if needed update the code to work with matplotlib > 3.4.

@hoxbro
Copy link
Member Author

hoxbro commented Apr 10, 2021

I have no idea why the unit test was skipped on Pytest on 3.8, windows-latest, maybe just a coincidence?

@hoxbro hoxbro changed the title Attempt at updating GitHub actions Updating GitHub actions Apr 11, 2021
@hoxbro hoxbro force-pushed the mac_conda_develop branch from ad894fb to 49ce22c Compare April 11, 2021 16:55
@hoxbro
Copy link
Member Author

hoxbro commented Apr 11, 2021

This PR should be ready for review. Some comment/notes since the initial PR:

  • I have updated the code to be compatible with matplotlib 3.4. This should fix Matplotlib 3.4.1 compatibility #4886.
  • I have also been wondering if the testing should be done only for python 3.7 and python 3.8, what are you thoughts on that?
  • For reason unknown python 3.8 on windows skips the unit test without failure.

@jlstevens
Copy link
Contributor

I've not done a detailed review yet but at a glance this PR looks good!

I have also been wondering if the testing should be done only for python 3.7 and python 3.8, what are you thoughts on that?

I think we should keep 3.6 tests at least (3.5 I am happy to ignore as that was a problematic release).

@philippjfr
Copy link
Member

philippjfr commented Apr 12, 2021

I think we should keep 3.6 tests at least (3.5 I am happy to ignore as that was a problematic release).

I think if we're dropping 2.7 we should also drop 3.5, it's been 5+ years and most libraries I know have dropped support for it at the same time as 2.7.

@philippjfr
Copy link
Member

Will merge. Thanks for your efforts @hoxbro!

@philippjfr philippjfr merged commit 52cd556 into holoviz:master Apr 13, 2021
@hoxbro hoxbro deleted the mac_conda_develop branch April 13, 2021 15:26
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants