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.

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

Error in leap year? #3464

Merged
merged 6 commits into from
Oct 30, 2019
Merged

Error in leap year? #3464

merged 6 commits into from
Oct 30, 2019

Conversation

mickaellalande
Copy link
Contributor

@mickaellalande mickaellalande commented Oct 30, 2019 •
edited
Loading

I've tried this script; however, it adds +1 to all months of the leap years. It sounds like an error, or I am wrong? So I wrote the condition "and month == 2" line 86 so that only the month of February gets +1.

  • Closes #xxxx
  • Tests added
  • Passes black . && mypy . && flake8
  • Fully documented, including whats-new.rst for all changes and api.rst for new API

christophcc and others added 3 commits October 14, 2019 12:57
@christophcc
Typo correction in docs (#3387)
e183c7a
@amcnicho @max-sixty
Update terminology.rst (#3455)
d197c81 
Fixed broken link
@mickaellalande
Error in leap year?
8d50626 
I've tried this script; however, it adds +1 to all months of the leap years. It sounds like an error, or I am wrong? So I wrote the condition "and month == 2" line 86 so that only the month of February gets +1.
@mickaellalande mickaellalande mentioned this pull request Oct 30, 2019
Closed
@max-sixty
Copy link
Collaborator

Hi @mickaellalande — thanks for the PR, happy to have you as an xarray contributor!

I think you're right.

I guess we don't have tests for these...

@mickaellalande because our docs branch (stable) is a bit behind master, tests are failing. I pushed a change to your code that branched off master. Could you please change the 'base' of your PR to master? Let me know if that's unclear.

Could you also add a change & your name to whatsnew?

@mickaellalande
Copy link
Contributor Author

Hi @max-sixty, thanks for the quick answer. It's my first contribution to Github repository other than mine so I'm not familiar with everything and git too. Can you detail me how to "change the 'base' of your PR to master" and where I "add a change & your name to whatsnew"?

Thanks in advance.

@keewis
Copy link
Collaborator

keewis commented Oct 30, 2019

Everyone has to start somewhere :)

To change the base, see here. Also, now that @max-sixty added some commits, you will probably have to git pull once before adding more changes yourself (otherwise it complains that the remote has different commits).

$repository/doc/whats-new.rst is a file where all the changes are listed.

@mickaellalande mickaellalande changed the base branch from stable to master October 30, 2019 13:28
@mickaellalande
Copy link
Contributor Author

I have no idea if I did the right thing... I changed the 'base' to master on this pull request and also on the repository : mickaellalande/xarray that was created. I don't know if had to do both? Let me know if I'm doing a mess...

I also changed the whatsnew from that repository (mickaellalande/xarray) and wrote: "Fix leap year (http://xarray.pydata.org/en/stable/examples/monthly-means.html) by Mickaël Lalande." under "Documentation".

@keewis
Copy link
Collaborator

keewis commented Oct 30, 2019 •
edited
Loading

this looks good to me, but I don't see your commit changing whats-new.rst. Did you push that commit?

Ignore the failure on windows, that is unrelated to this PR.

@mickaellalande
Copy link
Contributor Author

Should I update the whatsnewt on pydata/xarray or mickaellalande/xarray? I did it only on mine.

@keewis
Copy link
Collaborator

keewis commented Oct 30, 2019

yeah, you pushed to master on your own repository, but you should do that for patch-1

@keewis
Copy link
Collaborator

keewis commented Oct 30, 2019

that did it. @max-sixty, what do you think?

@mickaellalande
Copy link
Contributor Author

Ok I made it back to the branch "patch-1"

dcherian
dcherian reviewed Oct 30, 2019
View reviewed changes
doc/whats-new.rst Outdated Show resolved Hide resolved
@mickaellalande @dcherian
Update doc/whats-new.rst
563f966 
Co-Authored-By: Deepak Cherian <dcherian@users.noreply.github.com>
@dcherian
Copy link
Contributor

Great. Thanks @mickaellalande . Welcome to xarray!

@dcherian dcherian merged commit 59f88f7 into pydata:master Oct 30, 2019
@mickaellalande mickaellalande deleted the patch-1 branch October 30, 2019 15:52
@max-sixty
Copy link
Collaborator

Thanks @mickaellalande !

And @keewis for the mentoring!

@mickaellalande
Copy link
Contributor Author

mickaellalande commented Oct 31, 2019 •
edited
Loading

Thanks all of you guys to be patient and for your explanations :) Last question: can I archive or delete the mickaellalande/xarray repository on my account now?

@keewis
Copy link
Collaborator

keewis commented Oct 31, 2019

deleting won't affect this PR, so I'd say you are free to do what you want with your repository

dcherian added a commit to dcherian/xarray that referenced this pull request Nov 4, 2019
@dcherian
Merge remote-tracking branch 'upstream/master' into interp-na-maxgap
ac7aeeb 
* upstream/master:
  __dask_tokenize__ (pydata#3446)
  Type check sentinel values (pydata#3472)
  Fix typo in docstring (pydata#3474)
  fix test suite warnings re `drop` (pydata#3460)
  Fix integrate docs (pydata#3469)
  Fix leap year condition in monthly means example (pydata#3464)
  Hypothesis tests for roundtrip to & from pandas (pydata#3285)
  unpin cftime (pydata#3463)
  Cleanup whatsnew (pydata#3462)
  enable xr.ALL_DIMS in xr.dot (pydata#3424)
  Merge stable into master (pydata#3457)
  upgrade black verison to 19.10b0 (pydata#3456)
  Remove outdated code related to compatibility with netcdftime (pydata#3450)
  Remove deprecated behavior from dataset.drop docstring (pydata#3451)
  jupyterlab dark theme (pydata#3443)
  Drop groups associated with nans in group variable (pydata#3406)
  Allow ellipsis (...) in transpose (pydata#3421)
  Another groupby.reduce bugfix. (pydata#3403)
  add icomoon license (pydata#3448)
dcherian added a commit to dcherian/xarray that referenced this pull request Nov 8, 2019
@dcherian
Merge remote-tracking branch 'upstream/master' into fix/allow_lazy
6a6ba1b 
* upstream/master: (27 commits)
  drop_vars; deprecate drop for variables (pydata#3475)
  uamiv test using only raw uamiv variables (pydata#3485)
  Optimize dask array equality checks. (pydata#3453)
  Propagate indexes in DataArray binary operations. (pydata#3481)
  python 3.8 tests (pydata#3477)
  __dask_tokenize__ (pydata#3446)
  Type check sentinel values (pydata#3472)
  Fix typo in docstring (pydata#3474)
  fix test suite warnings re `drop` (pydata#3460)
  Fix integrate docs (pydata#3469)
  Fix leap year condition in monthly means example (pydata#3464)
  Hypothesis tests for roundtrip to & from pandas (pydata#3285)
  unpin cftime (pydata#3463)
  Cleanup whatsnew (pydata#3462)
  enable xr.ALL_DIMS in xr.dot (pydata#3424)
  Merge stable into master (pydata#3457)
  upgrade black verison to 19.10b0 (pydata#3456)
  Remove outdated code related to compatibility with netcdftime (pydata#3450)
  Remove deprecated behavior from dataset.drop docstring (pydata#3451)
  jupyterlab dark theme (pydata#3443)
  ...
@keewis keewis mentioned this pull request Nov 21, 2019
Merged
1 task
keewis pushed a commit to keewis/xarray that referenced this pull request Nov 21, 2019
@mickaellalande @dcherian @keewis
Fix leap year condition in monthly means example (pydata#3464)
1faf67c 
* Typo correction in docs (pydata#3387)

* Update terminology.rst (pydata#3455)

Fixed broken link

* Error in leap year?

I've tried this script; however, it adds +1 to all months of the leap years. It sounds like an error, or I am wrong? So I wrote the condition "and month == 2" line 86 so that only the month of February gets +1.

* Fix leap year (pydata#3464)

* Update doc/whats-new.rst

Co-Authored-By: Deepak Cherian <dcherian@users.noreply.github.com>
dcherian pushed a commit that referenced this pull request Nov 22, 2019
@keewis @dcherian
Switch examples to notebooks + scipy19 docs improvements (#3557)
b0064b2 
* Switch doc examples to use nbsphinx (#3105)

* switching out examples to use nbsphinx

* added jupyter_client to doc env

* added ipykernel to doc env

* Replace sphinx_gallery with notebook (#3106)

* switching out examples to use nbsphinx

* added jupyter_client to doc env

* moved gallery to notebook

* Allow other tutorial filename extensions (#3121)

* switching out examples to use nbsphinx

* added jupyter_client to doc env

* allow non netcdf tutorial files

* Added ROMS ocean model example notebook (#3116)

* change name of test env to xarray-tests (#3110)

* ROMS_ocean_model example added

* Allow other tutorial filename extensions (#3121)

* switching out examples to use nbsphinx

* added jupyter_client to doc env

* allow non netcdf tutorial files

* Changed load to xr.tutorial.open_dataset(), and
added some extra documentation.

* change name of test env to xarray-tests (#3110)

* ROMS_ocean_model example added

* Changed load to xr.tutorial.open_dataset(), and
added some extra documentation.

* fixed colormap issues leftover from cmocean import

* Added intro paragraph to ROMS example notebook, removed comments, and added citation in whats-new.

* Add an example of ERA5 and GRIB data & visualization to the gallery (#3199)

* Adds an example of ERA5 and GRIB data to the gallery

* Add markdown narrative cells to GRIB example

* Update load method to use xr.tutorial

* Fix load method

* require nbsphinx for the documentation builds

* add more nbsphinx dependencies

* install cfgrib using pip

* add the eccodes library to the dependencies

* remove the dependency on sphinx-gallery

* add the ERA5 GRIB example to the list

* update the documentation links

Missing: section links in visualization_gallery.ipynb don't work yet,
also the one in io.rst (it has a unicode char).

* Fix leap year condition in monthly means example (#3464)

* Typo correction in docs (#3387)

* Update terminology.rst (#3455)

Fixed broken link

* Error in leap year?

I've tried this script; however, it adds +1 to all months of the leap years. It sounds like an error, or I am wrong? So I wrote the condition "and month == 2" line 86 so that only the month of February gets +1.

* Fix leap year (#3464)

* Update doc/whats-new.rst

Co-Authored-By: Deepak Cherian <dcherian@users.noreply.github.com>

* fix the reference to the rasterio geocoordinates docs

* update whats-new.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcherian dcherian dcherian left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mickaellalande @max-sixty @keewis @dcherian @christophcc @amcnicho