Skip to content

Persist calendar when discarding cftime microsecond #183

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.

Sign up for GitHub

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

Jump to bottom
Merged
rcomer merged 3 commits into from
Jun 30, 2021
Merged

Persist calendar when discarding cftime microsecond #183

rcomer merged 3 commits into from
Jun 30, 2021

Conversation

@bjlittle
Copy link
Member

@bjlittle bjlittle commented Jun 29, 2021 •
edited
Loading

🚀 Pull Request

Description

This PR addresses issues discovered whilst testing SciTools/iris when attempting to unpin cftime, whereby the specific calendar of cftime.datetime objects are not persisted whenever the cf_units._discard_microsecond function was called.

Once this PR is merged, I'd like to cut a cf-units 3.0.1 patch release to support an iris 3.0.3 patch release.

@bjlittle bjlittle added Type: Bug Release: Patch New: Pull Request Highlight a new community raised pull-request labels Jun 29, 2021
bjlittle
bjlittle commented Jun 29, 2021
View reviewed changes
@codecov
Copy link

codecov bot commented Jun 29, 2021 •
edited
Loading

Codecov Report

Merging #183 (528be8a) into v3.0.x (5475a85) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           v3.0.x     #183   +/-   ##
=======================================
  Coverage   90.63%   90.63%           
=======================================
  Files           7        7           
  Lines         833      833           
  Branches      105      105           
=======================================
  Hits          755      755           
  Misses         64       64           
  Partials       14       14
Impacted Files Coverage Δ
cf_units/__init__.py 89.58% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5475a85...528be8a. Read the comment docs.

bjlittle
bjlittle commented Jun 29, 2021
View reviewed changes
bjlittle
bjlittle commented Jun 29, 2021
View reviewed changes
pyproject.toml
| build
| dist
)/
| _version.py
Copy link
Member Author

@bjlittle bjlittle Jun 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is auto-generated by setuptools-scm and should not be linted

Copy link
Member

@rcomer rcomer Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full disclosure: I don't understand this file. But if it's auto-generated and @bjlittle is happy with it then it should be 👍

@bjlittle bjlittle requested a review from pp-mo June 29, 2021 22:04
@bjlittle bjlittle changed the title persist calendar when discarding cftime microsecond Persist calendar when discarding cftime microsecond Jun 30, 2021
rcomer
rcomer reviewed Jun 30, 2021
View reviewed changes
@rcomer rcomer self-assigned this Jun 30, 2021
rcomer
rcomer reviewed Jun 30, 2021
View reviewed changes
Copy link
Member

@rcomer rcomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bjlittle I have one comment for your consideration. I've not yet looked at the test module as it seems likely that the outcome of the comment may change the tests.

@rcomer rcomer merged commit 5aa890a into SciTools:v3.0.x Jun 30, 2021
rcomer pushed a commit that referenced this pull request Jul 24, 2021
@bjlittle
Persist calendar when discarding cftime microsecond (#183) (#192)
d9f280c 
* persist calendar when discarding cftime microsecond

* review actions

* review actions
@bjlittle bjlittle deleted the fix-discard-microseconds-calendar branch July 12, 2022 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rcomer rcomer rcomer approved these changes

@pp-mo pp-mo Awaiting requested review from pp-mo

Assignees

@rcomer rcomer

Labels

New: Pull Request Highlight a new community raised pull-request Release: Patch Type: Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bjlittle @rcomer