modified query index for compatibility with pandas by zztin · Pull Request #9 · MarvinT/calmap

zztin · 2020-08-10T16:56:59Z

In this fix 2 issues are addressed:

Tests failing on current version (216fc6b, tag: v0.0.8): ../calmap/__init__.py:238 KeyError
Details please see pytest log: https://pastebin.ubuntu.com/p/TQt7WRFzpc/
Future warning
current_projects/calmap/calmap/init.py:181: FutureWarning: weekofyear and week have been deprecated, please use DatetimeIndex.isocalendar().week instead, which returns a Series. To exactly reproduce the behavior of week and weekofyear and return an Index, you may call pd.Int64Index(idx.isocalendar().week)
"week": by_day.index.week,

My running environment:

Python 3.8.3
Pandas 1.1.0

Discussions and comments are very welcome!

zztin · 2021-01-07T11:58:01Z

Imcompatible with python 2.7 and 3.5, passed with python 3.6+ due to pandas version bump to v1.0. Would you consider merging the pull request?

ErikBjare · 2021-01-31T16:20:20Z

These changes look great, having similar issues with recent pandas. I hope dropping Python 2.7 support isn't an issue (shouldn't be anyway).

@MarvinT Any chance this'll be merged?

ErikBjare · 2021-01-31T16:21:30Z

+                       norm=norm,
+                       orientation='horizontal',
+                       ticks=range(vmin, vmax + 1))
+    cb1.set_label('CTimer clocks')


This label looks a bit weird, is it correct?

It's removed in the current PR.

MarvinT

Hey @zztin , sorry I didn't get to this earlier.

Thanks for updating the package. I am comfortable removing support for 2.7 and 3.5
Any idea why 3.7 tests are failing?
We'll have to change .travis.yml to only push to the compatible py versions because right now I have it push as a universal build.
Can you also format using black?

I've added a few comments inline as well

MarvinT · 2021-02-14T06:22:51Z

-
+__author__ = "Marvin Thielk; Martijn Vermaat; Liting Chen"
+__contact__ = "marvin.thielk@gmail.com, martijn@vermaat.name, litingchen16@gmail.com"
+__homepage__ = "https://github.com/zztin/calmap"


I don't mind adding you as a contact but can you leave the homepage pointing to my version since that's what pypi uses.

I have reverted the changes. This was a mistake of mine to include in this PR :)

MarvinT · 2021-02-14T06:26:18Z

    how="sum",
    vmin=None,
    vmax=None,
-    cmap="Reds",


Is there a reason to change the default behavior here? its a kwarg so if anyone wants reds they can use it but this will change everyone's plots without documenting the changes anywhere...

I will revert the change back to its default behavior.

MarvinT · 2021-02-14T06:32:15Z

+    vmin=0,
+    vmax=16,


I feel like the defaults should be None

I agree. It has been changed to None.

MarvinT · 2021-02-14T06:33:04Z

    ylabel_kws = dict(
        fontsize=32,
-        color=kwargs.get("fillcolor", "whitesmoke"),
+        color=kwargs.get("fillcolor", "silver"),


what is this change for?

It has been removed from the current PR.

MarvinT · 2021-02-14T06:48:39Z

+    cmap = plt.get_cmap(cmap)
+    bounds = range(vmin, vmax +1)
+    norm = BoundaryNorm(bounds, cmap.N, extend='both')
+
+    cb1 = ColorbarBase(axes[-1],
+                       cmap=cmap,
+                       norm=norm,
+                       orientation='horizontal',
+                       ticks=range(vmin, vmax + 1))


adding a colorbar should be an optional thing. I personally prefer the clean look of not having a color bar.

It has been removed from this pull request. I am planning to send another pull request regarding the option to include a heatmap at a later stage.

lawuka · 2021-03-04T16:37:19Z

I am also looking for a fix to these two exact issues and I would prefer if the colorbar and other custom changes were left out as @MarvinT is also questioning. Are you still up for making the changes @zztin ?

zztin · 2021-03-08T12:31:46Z

Hi all! Yes, I will look into them asap (before next week). Sorry for the delay! Best, Liting

…

On Fri, Mar 5, 2021 at 12:37 AM Alexander Kvist Andreasen < ***@***.***> wrote: I am also looking for a fix to these two exact issues and I would prefer if the colorbar and other custom changes were left out as @MarvinT <https://github.com/MarvinT> is also questioning. Are you still up for making the changes @zztin <https://github.com/zztin> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AH2QX2L4R6ZL32XAIPMZ5GLTB6ZNBANCNFSM4P2E4EAA> .

fakhavan · 2021-03-10T02:06:41Z

Hi everyone!
I just wanted to add that I get this error when I was using your library inside one of our notebooks on Google Colab:

This issue goes away when you install pandas 1.0.5 on colab and restart the kernel (runtime).
However wanting to find a better solution, I tried it with zztin:master with the pre-installed pandas on colab (it's version 1.1.5) and I still got an error at the exact same part with a different error message:

I'm not sure if this issue is related to colab only or happens on a local environment with the same package versions but I thought it could be useful if you know and consider it in your next update.
Thank you

Tests failed with KeyError: datetime.date(2014, 1, 15) in python 3.6+ for pandas compatibility. Also fixed: calmap/init.py:181: FutureWarning: weekofyear and week have been deprecated, please use DatetimeIndex.isocalendar().week instead

Add flexibility for plotting style: option to add border per month and plot title.

tai271828 · 2021-03-11T13:27:00Z

Python 3.7 failure of Travis CI is caused by numpy version,

...... (skipped messages)......
Requirement already satisfied: numpy>=1.15 in /home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages (from matplotlib>=2.0.0->-r doc/requirements.txt (line 1)) (1.16.4)
...... (skipped messages)......
ERROR: pandas 1.2.3 has requirement numpy>=1.16.5, but you'll have numpy 1.16.4 which is incompatible.
...... (skipped messages)......
ImportError: this version of pandas is incompatible with numpy < 1.16.5
346your numpy version is 1.16.4.
347Please upgrade numpy to >= 1.16.5 to use this pandas version

The travis environment shows some specific versions are used. Let me have a look if they raise version dependency issue:

# .travis.yml
install:
- pip install -r tests/requirements.txt -r doc/requirements.txt
- pip install -e .

and the corresponding requirement file content:

# test/requirements.txt
pytest>=4.6
coverage
pytest-cov
python-coveralls

# doc/requirements.txt
pytest>=4.6
coverage
pytest-cov
python-coveralls
matplotlib>=2.0.0
numpydoc
Sphinx<=1.8.2

I built from scratch for the original master (https://github.com/MarvinT/calmap) with the above requirement files and follow the pip install part of travis.yml, and I did not get the same error. The dependency issue should be raised by the other steps of travis.

Besides, I also run the original master travis CI[*], and the original master (https://github.com/MarvinT/calmap) travis config seems outdated a bit and not all python version will pass. My gut feeling shows this is for the same version dependency issue.

[*] https://travis-ci.com/github/tai271828/calmap/builds/219751194

When people were working on this pull request MarvinT#9 , we were aware of the numpy version compatibility issue during CI. This commit would fix the issue by using numpy with explicit version.

tai271828 · 2021-03-11T14:35:12Z

I fixed the Travis CI error based on this pull request with the pull request #10 . Please feel free to land my pull request, or cherry-pick it in this pull request if you want.

When people were working on this pull request MarvinT#9 , we were aware of the numpy version compatibility issue during CI. This commit would fix the issue by using numpy with explicit version.

zztin · 2021-03-11T15:59:38Z

Hi all,
The pull request has been updated to include the basic changes essential for pandas support and made style changes optional. I also excluded all customized modification that was accidentally committed into the previous PR request.

Thank you @tai271828 for the Travis CI fix. I've included the changes listed in #10 in this PR.

I have not taken into account the remark related to Colab notebook. Maybe the author could allow the issue page for submitting other issues?

fakhavan · 2021-03-11T20:52:11Z

I just tested the latest zztin:master with Colab and the issues have been fixed. I think the issue was not anything different than an incompatibility between the latest versions of pandas and calmap which seems to be fixed with these changes

ErikBjare · 2021-03-22T06:57:40Z

@MarvinT This looks ready to merge! 🎉

MarvinT

Alright, thanks for all the work. I think the only thing left is updating the version number and I'll deploy it.

Co-authored-by: Erik Bjäreholt <erik.bjareholt@gmail.com>

MarvinT · 2021-03-24T01:41:36Z

and released! thanks all!

ErikBjare reviewed Jan 31, 2021

View reviewed changes

MarvinT requested changes Feb 14, 2021

View reviewed changes

zztin added 2 commits March 11, 2021 20:21

Fixed compatibility with pandas

2e1a698

Tests failed with KeyError: datetime.date(2014, 1, 15) in python 3.6+ for pandas compatibility. Also fixed: calmap/init.py:181: FutureWarning: weekofyear and week have been deprecated, please use DatetimeIndex.isocalendar().week instead

New features: Figure title, Style Option

caf12eb

Add flexibility for plotting style: option to add border per month and plot title.

zztin force-pushed the master branch from 77d346d to caf12eb Compare March 11, 2021 12:54

tai271828 mentioned this pull request Mar 11, 2021

CI house cleaning #10

Closed

tai271828 added 2 commits March 11, 2021 23:49

Stop supporting Python 2.7 and 3.5

5f29103

Fix CI error for Python 3.7.

1af75e3

When people were working on this pull request MarvinT#9 , we were aware of the numpy version compatibility issue during CI. This commit would fix the issue by using numpy with explicit version.

Apply black formatting

1e18319

MarvinT reviewed Mar 23, 2021

View reviewed changes

Comment thread calmap/__init__.py

Comment thread calmap/__init__.py

Comment thread calmap/__init__.py Outdated

Update version number

8bfbad3

Co-authored-by: Erik Bjäreholt <erik.bjareholt@gmail.com>

MarvinT approved these changes Mar 24, 2021

View reviewed changes

MarvinT merged commit 336a6e2 into MarvinT:master Mar 24, 2021

Conversation

zztin commented Aug 10, 2020

Uh oh!

zztin commented Jan 7, 2021

Uh oh!

ErikBjare commented Jan 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarvinT left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zztin Mar 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lawuka commented Mar 4, 2021

Uh oh!

zztin commented Mar 8, 2021 via email

Uh oh!

fakhavan commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tai271828 commented Mar 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tai271828 commented Mar 11, 2021

Uh oh!

zztin commented Mar 11, 2021

Uh oh!

fakhavan commented Mar 11, 2021

Uh oh!

ErikBjare commented Mar 22, 2021

Uh oh!

MarvinT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MarvinT commented Mar 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ErikBjare commented Jan 31, 2021 •

edited

Loading

zztin Mar 11, 2021 •

edited

Loading

fakhavan commented Mar 10, 2021 •

edited

Loading

tai271828 commented Mar 11, 2021 •

edited

Loading