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: Workaround for Pandas Timestamp.isoformat issue #17426

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

john-bodley
Copy link
Member

@john-bodley john-bodley commented Nov 12, 2021

SUMMARY

The Pandas timestamp differs from the datetime.datetime object and the isoformat function doesn't include the timespec argument (the help description is wrong), i.e., it results in errors of the form,

TypeError: isoformat() got an unexpected keyword argument 'timespec

See pandas-dev/pandas#26131 for more details. The fix is to convert the pandas.TImestampobject into adatetime.datetime` object which is consistent with the existing logic, even though

>>> isinstance(pd.Timestamp('2021-11-12'), datetime.datetime)
True

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

CI and tested manually, i.e., checked the return type.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@john-bodley john-bodley changed the title fix: Pandas timestamp fix: Workaround for Pandas Timestamp.isoformat issue Nov 12, 2021
@codecov
Copy link

codecov bot commented Nov 12, 2021

Codecov Report

Merging #17426 (8e5b0ea) into master (bcef8fa) will decrease coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17426      +/-   ##
==========================================
- Coverage   76.96%   76.82%   -0.14%     
==========================================
  Files        1041     1041              
  Lines       56066    56066              
  Branches     7738     7738              
==========================================
- Hits        43150    43073      -77     
- Misses      12658    12735      +77     
  Partials      258      258              
Flag Coverage Δ
hive ?
javascript 71.22% <ø> (ø)
mysql 81.93% <100.00%> (ø)
postgres 81.94% <100.00%> (ø)
python 82.03% <100.00%> (-0.27%) ⬇️
sqlite 81.62% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/utils/date_parser.py 96.63% <100.00%> (ø)
superset/db_engines/hive.py 0.00% <0.00%> (-85.19%) ⬇️
superset/db_engine_specs/hive.py 69.49% <0.00%> (-16.99%) ⬇️
superset/views/database/mixins.py 81.03% <0.00%> (-1.73%) ⬇️
superset/db_engine_specs/presto.py 83.47% <0.00%> (-0.84%) ⬇️
superset/db_engine_specs/base.py 88.20% <0.00%> (-0.39%) ⬇️
superset/connectors/sqla/models.py 86.35% <0.00%> (-0.24%) ⬇️
superset/utils/core.py 89.98% <0.00%> (-0.13%) ⬇️

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 bcef8fa...8e5b0ea. Read the comment docs.

@john-bodley john-bodley merged commit bfc813d into master Nov 12, 2021
AAfghahi pushed a commit that referenced this pull request Jan 10, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024
@mistercrunch mistercrunch deleted the john-bodley--fix-date-parse branch March 26, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants