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

fix pd.to_datetime inferring date incorrectly #13

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

danieleongari
Copy link
Contributor

Hi,
thanks for making this nice python API!

I'm fixing a small bug: when data of TIME_PERIOD is presented as integers, e.g., for years, the pandas function to_datetime infer incorrectly the value as microseconds from the reference datetime 1970-01-01-00:00:00.000. To fix this, I simply convert the number to string.

Working fine with pandas==1.5.3.

@Attol8
Copy link
Owner

Attol8 commented Mar 20, 2023

Hi @danieleongari thanks for the fix! looks good to me and it's a change that we need.

However, as the project is developed with nbdev, we need some additional steps before pushing your changes to Github to clean the content of the notebook and avoid conflicts. without these steps, the CI tests will fail. My bad, as I should have had some contributing guidelines in the readme file. will add it in the near future.

Before pushing to Github, can you make sure to:

  1. Clone the repository
  2. install nbdev with pip install nbdev
  3. run nbdev_install_hooks
  4. Most importantly, the library is built using the notebooks contained in the nbs folder. The .py file that you modified is autogenerated so we cannot edit it. If you want to make any changes to the library you have to find the relevant notebook and make your changes there. In your case it would require making changes to nbs\03_retrieval.ipynb
  5. Once changes are made, run nbdev_prepare and make sure that all tests are passing
  6. commit and push your changes to Github

Let me know if you have difficulties running any of these steps and looking forward to accept your PR :)

@danieleongari
Copy link
Contributor Author

Ok, no problem, I will do it ASAP. I was not aware of this way of translating a notebook into a package, but I'm happy to learn!

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@danieleongari
Copy link
Contributor Author

Followed the instructions, hope it works fine!

@Attol8 Attol8 merged commit e4c8d9f into Attol8:master Mar 21, 2023
@Attol8
Copy link
Owner

Attol8 commented Mar 21, 2023

@danieleongari thanks a lot for the changes, all tests are passing now 😄pr has been merged

Let me know if you find the library useful and what use cases you have so that I can open new issues for people interested in participating!! Could be a useful project for anyone looking to explore open-source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants