search

grambank / pygrambank

Apache License 2.0
4 stars 1 forks source link

Quality of life things for hedvig #57

Closed HedvigS closed 2 years ago

HedvigS commented 2 years ago

Just a few small changes that would make the grambank PR-review process easier for me

HedvigS commented 2 years ago

Could you address the failing test - and preferably run the tests locally before commit? The problem seems to be the changed order of cli arguments for sourcelookup.

I saw that tests failed but I couldn't figure out what was going on. I'm not well-versed enough in this set-up. I can try, but yeah. Not confident.

HedvigS commented 2 years ago

Haha, made it worse :)

xrotwang commented 2 years ago

Why would you want to change the order of cli args? That's a change that would impact anyone else running the commands as well.

xrotwang commented 2 years ago

Did you try running the tests locally? All you'd need to do would be

pip install -e .[dev,test]
pytest
HedvigS commented 2 years ago

Why would you want to change the order of cli args? That's a change that would impact anyone else running the commands as well.

Because it makes my life easier when I need to run several sheets.

Who else but you and I are using these commands?

xrotwang commented 2 years ago

Right. So you'd change things for me.

HedvigS commented 2 years ago

Right. So you'd change things for me.

Yes. Is that ok?

HedvigS commented 2 years ago

It makes it easier for me as I'm going through and running sourcelookup for many sheets if the sheet argument is last because then i can just backspace a few characters and change the sheet name.

xrotwang commented 2 years ago

I see. It makes things harder for me, because if I come back to having to run the command I have to remember things changed. Anyway, since you are taking over triaging PRs, you can have it your way.

HedvigS commented 2 years ago

I see. It makes things harder for me, because if I come back to having to run the command I have to remember things changed. Anyway, since you are taking over triaging PRs, you can have it your way.

Thanks that's very kind of you.

I've got a document called "pygrambank notes" where i write up specific help for myself, such as the order of arguments etc :)

In R, you can either rely on the order of arguments for a function or specify the arguments specifically. If you do the second, you don't need to conform to the default order. I find that is often very helpful.