BUG: parsed datetime string resolution incorrect

[boringow]

I was working today with datetime index, you have the reso (resolutions) defined from year to nanosecond, but then you had the lines 560-570 repeating 'minute' and 'second', and working with millisecond or nanosecond raises a KeyIndex error.

• closes CLN remove duplicated entries in valid_resos in pandas/core/indexes/datetimes.py #38121
• closes BUG: raise consistent exception on slicing failure #38077
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[pep8speaks]

Hello @boringow! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file pandas/core/indexes/datetimes.py:

Line 572:1: W293 blank line contains whitespace

[arw2019]

Can you add test(s)? That's the first thing we look for when reviewing

[boringow]

Hi @arw2019 , I have only 4 months of programming experience, and not more than 3 weeks of using Github. By adding a test you mean adding a small example? can I copy paste it here on the comments?

[arw2019]

Hi @arw2019 , I have only 4 months of programming experience, and not more than 3 weeks of using Github.

Welcome to pandas! We encourage new contributors and we'll guide you through the process of completing the pull request.

By adding a test you mean adding a small example? can I copy paste it here on the comments?

Can I suggest that you open a github issue with the small example and the output you expect? Reference this pull request in the issue and we'll take it from there.

[boringow]

Okay! I am going to do so 👍

[boringow]

done :)

[jreback]

can you add a test for this that fails on master and this fixes

[boringow]

I added the test on the issue related to this bug, I tagged you in there, that example is from the master, and this code fixes.

[arw2019]

Can you add "- Closes #38121" to the description at the top of this PR? Then GitHub will link this to the issue

As per #38121 (comment) the snippet you provided is too long/complicated - we need the reproducer to be simple and concise. Please trim it down to localize the bug you're fixing.

Once you have a small reproducer, you will need to add that test to our testing suite, in the pandas/tests folder

[MarcoGorelli]

Hi @boringow - is this still active? If so, could you please address the comments above. First things first, you'll need to write a test - see the contributing guide

pandas is serious about testing and strongly encourages contributors to embrace test-driven development (TDD). This development process “relies on the repetition of a very short development cycle: first the developer writes an (initially failing) automated test case that defines a desired improvement or new function, then produces the minimum amount of code to pass that test.” So, before actually writing any code, you should write your tests. Often the test can be taken from the original GitHub issue. However, it is always worth considering additional use cases and writing corresponding tests.

Adding tests is one of the most common requests after code is pushed to pandas. Therefore, it is worth getting in the habit of writing tests ahead of time so this is never an issue.

Like many packages, pandas uses pytest and the convenient extensions in numpy.testing.

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

[arw2019]

@boringow if you'd like to pick this up the process is:

1. write a unit test. This should be a code snippet that fails on master but passes after your patch. Add the test to a location in pandas/testing directory that houses similar tests
2. come up with a fix - that's done

we'll then circle back to review and when the patch is approved we'll merge.

Closing for now as this doesn't seem active but ping us if you'd like to pick this up and we'll reopen!

[boringow]

Is it finally closed? I did a test and everything requested, but you requested more. El dom, 24 ene 2021 a las 2:22, Andrew Wieteska (<[email protected]>) escribió:

…

Closed #38118 <#38118>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#38118 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOZXKJVYWY7KIF6DWRI7PLDS3NY4ZANCNFSM4UFFJJYQ> .