search

rs-station / reciprocalspaceship

Tools for exploring reciprocal space
https://rs-station.github.io/reciprocalspaceship/
MIT License
28 stars 12 forks source link

Update `rs.DataSet.reset_index()` call signature to matches `pandas >v1.5` #266

Closed JBGreisman closed 1 month ago

JBGreisman commented 1 month ago

There was an update to the pandas API in v1.5 that changed the pd.DataFrame.reset_index() call signature. This PR updates the call signature of rs.DataSet.reset_index() and adds tests to confirm the consistency of the call signature between:

JBGreisman commented 1 month ago

I'm open to the idea of the *args/**kwargs paradigm and we do use it elsewhere. Personally, I find these to be very frequently used methods, so I enjoy having the easy introspection of call signature and correct docstrings (that don't mention pandas specifics). That was the main rationale for keeping it this way. Definitely open to reconsidering if you feel otherwise though.

kmdalton commented 1 month ago

I'm open to the idea of the *args/**kwargs paradigm and we do use it elsewhere. Personally, I find these to be very frequently used methods, so I enjoy having the easy introspection of call signature and correct docstrings (that don't mention pandas specifics). That was the main rationale for keeping it this way. Definitely open to reconsidering if you feel otherwise though.

I think there are good reasons not to use *args/ **kwargs in this case. It's quite an important method. I was just trying to safeguard against changes in Pandas breaking the CI. I think maybe the CI should break in this case. We should hear about it whenever their call signature changes. I'm good with this.