search

xcube-dev / xcube-cds

An xcube plugin to generate data cubes from the Climate Data Store (CDS) API
MIT License
3 stars 1 forks source link

Update code to work with new xcube type specifiers #21

Closed pont-us closed 3 years ago

pont-us commented 3 years ago

Closes #20 .

pont-us commented 3 years ago

Looks mostly good, but please consider not separating imports by commas but instead using one import per line.

Fixed throughout.

Also, there were a lot of line breaks which did not seem necessary to me.

I try to keep xcube-cds code style compliant with the xcube developer guide, which recommends PEP 8 style, which recommends a maximum line length of 79 characters. I break lines only when this limit is exceeded.

pont-us commented 3 years ago

Note that Travis builds for this PR will fail until https://github.com/dcs4cop/xcube/pull/359 is merged, since the xcube-cds Travis config tests with the head of the xcube master branch.

TonioF commented 3 years ago

I try to keep xcube-cds code style compliant with the xcube developer guide, which recommends PEP 8 style, which recommends a maximum line length of 79 characters. I break lines only when this limit is exceeded.

Interesting. I think I got fooled by the PyCharm messag that line breaks should occur after 120 characters here ( see here: https://stackoverflow.com/questions/26808681/why-does-pycharm-use-120-character-lines-even-though-pep8-specifies-79 ) . I'd love if we could agree on 99 characters for code, as PEP8 allows at max.

pont-us commented 3 years ago

I'd love if we could agree on 99 characters for code, as PEP8 allows at max.

Personally I'm an 80-column fundamentalist (in part because I love having the horizontal space for a side-by-side diff and some additional IDE panels), but if there's a democratic majority for 99 columns and if we officially mandate it in the xcube developer guide, I will comply :).