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

Drop dateutil dependency #2173

Merged
merged 4 commits into from
Feb 10, 2025
Merged

Drop dateutil dependency #2173

merged 4 commits into from
Feb 10, 2025

Conversation

knyghty
Copy link
Contributor

@knyghty knyghty commented Feb 8, 2025

What does this change

Removes dateutil.

What was wrong

Nothing really. Dateutil is a bit less maintained and the use of it here can be done fairly easily with the standard library.

How this fixes it

Fixes #2168

Replaced usage of dateutil with stlib stuff. Changes should be covered by existing tests. Had to change one test that was referencing dateutil directly.

Could maybe argue this is backwards-incompatible if people are doing isinstance checks or otherwise doing something weird, but maybe that is going a bit far.

Checklist

  • I have read the documentation about CONTRIBUTING
  • I have read the documentation about Coding style
  • I have run make lint

Sorry, something went wrong.

@knyghty
Copy link
Contributor Author

knyghty commented Feb 8, 2025

This will need rebasing after #2172 is merged.

Comment on lines 2342 to 2345
if this_month_start.month == 12:
next_month_start = this_month_start.replace(year=this_month_start.year + 1, month=1)
else:
next_month_start = this_month_start.replace(month=this_month_start.month + 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should abstract this logic into a function that can be reused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, was thinking about this as well, will have a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've done this (possibly could have done more, but would be rather substantial and possibly illegible so I stopped myself).

I also removed install_requires since it's now empty after this change and the Py38 removal.

fcurella and others added 3 commits February 10, 2025 11:33
Copy link
Collaborator

@fcurella fcurella left a comment

Choose a reason for hiding this comment

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

Thank you!

AwesomeCodes

@fcurella fcurella merged commit 604e1ed into joke2k:master Feb 10, 2025
28 checks passed
@knyghty knyghty deleted the drop-dateutil branch February 10, 2025 18:22
@g-as
Copy link

g-as commented Feb 10, 2025

It is still listed as a dependency after v36.0.0 release:

Metadata-Version: 2.1
Name: Faker
Version: 36.0.0

[...]

Classifier: Development Status :: 5 - Production/Stable
Classifier: Environment :: Console
Classifier: Intended Audience :: Developers
Classifier: Programming Language :: Python :: 3.9
Classifier: Programming Language :: Python :: 3.10
Classifier: Programming Language :: Python :: 3.11
Classifier: Programming Language :: Python :: 3.12
Classifier: Programming Language :: Python :: 3.13
Classifier: Programming Language :: Python :: Implementation :: CPython
Classifier: Programming Language :: Python :: Implementation :: PyPy
Classifier: Topic :: Software Development :: Libraries :: Python Modules
Classifier: Topic :: Software Development :: Testing
Classifier: Topic :: Utilities
Classifier: License :: OSI Approved :: MIT License
Requires-Python: >=3.9
License-File: LICENSE.txt
Requires-Dist: python-dateutil>=2.4

I'm not really sure why...

@fcurella
Copy link
Collaborator

Sorry, my mistake. It's been removed in v36.1.0

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.

Drop dateutil dependency
3 participants