Skip to content

Conversation

WarrenWeckesser
Copy link
Member

Issue gh-2880 has been open since 2013. In a recent community meeting, it was suggested that we create a NEP to propose the removal of the financial functions from NumPy. This is the initial draft of that NEP.

@rgommers
Copy link
Member

Nice proposal @WarrenWeckesser, LGTM

@mattip
Copy link
Member

mattip commented Aug 30, 2019

The content here looks very complete and it not only summarizes the issue but clearly lays out a path forward. It does not comply with the NEP template section headers as required in the nep process document, do we care in this case?

@WarrenWeckesser
Copy link
Member Author

@mattip wrote

It does not comply with the NEP template section headers as required in the nep process document, do we care in this case?

Yes! I'll fix that.

@WarrenWeckesser
Copy link
Member Author

I pushed an update to comply with the NEP template section headings. I didn't include the sections "Detailed description" or "Related work". I think the "Implementation" section gives all the details needed (the actual change is pretty simple), and a section on "Related work" is not relevant to this NEP.

@seberg
Copy link
Member

seberg commented Aug 30, 2019

Nice, it will be great to just have a doc to point to when discussion starts after we put the deprecation warnings in. (I am not sure if a lot of the single backticks need to be double backticks).
Do we have to make clear that numpy_financial is not currently planned to be actively maintained (the only reason would be to be clear up front that it is a transitional package until a clear alternative comes around)?

@WarrenWeckesser
Copy link
Member Author

@seberg wrote:

(I am not sure if a lot of the single backticks need to be double backticks).

Fixed.

@WarrenWeckesser
Copy link
Member Author

@seberg wrote:

Do we have to make clear that numpy_financial is not currently planned to be actively maintained

I'm not sure. How pertinent is that to the NEP? If someone wants to propose appropriate wording, I can add it (or I can accept a PR).

(the only reason would be to be clear up front that it is a transitional package until a clear alternative comes around)?

What is the significance of referring to it as a "transitional" package? If a clear alternative arises, we can certainly help promote it, but I don't think we would ever have to remove the numpy_financial library. For many users who need only the simplest functions, numpy_financial wouldn't necessarily be transitional. It might turn out that a clearly better alternative arises in the future, and perhaps the new library will be able to take responsibility for numpy_financial, but I don't think we can say we are counting on that happening, so I don't think we should plan on eventually removing numpy_financial.

Also, while I don't think anyone is suggesting this, I should make clear that I do not think we should intentionally not maintain numpy_financial. The interests and experience of the next generation of NumPy developers might be different, and it might make sense for them to actively improve numpy_financial. In the short term, however, we would probably have to recommend to potential contributors to numpy_financial that they recruit additional domain experts to assist with review.

@seberg
Copy link
Member

seberg commented Aug 30, 2019

The purpose from my side was to make it clear, in documentation, that we encourage outsiders to pick it up (or better replace it). If we are tempted to write in the numpy_financial documentation "we may not be really be maintaining this", I thought we might want to say that up-front now, the question may come up in any case. I think the name numpy_financial is pretty clear that it is not anticipated to grow, but rather to be replaced at some point.
Anyway, really just thinking out loud. I think the current NEP reads nice, and we can probably propose it to the list.

@WarrenWeckesser
Copy link
Member Author

OK, that makes sense. I think we'd all be happy if a dedicated group of domain experts eventually took over responsibility for numpy_financial. Should that be mentioned in the NEP? Or just in the documentation of numpy_financial once it is created? Or somewhere else...?

@WarrenWeckesser
Copy link
Member Author

After thinking about @seberg's comment some more, I don't think any additional comments are needed in the NEP. If anyone disagrees, can you propose specific wording?

Otherwise, I think the NEP should be merged.

@seberg
Copy link
Member

seberg commented Sep 8, 2019

I am OK with merging this either way. The intent is clear and how it fits into things is as well. Even if there may be clarifications, or so, none of that will make it will likely change the impression a lot, or what the decision on it will be.
If no one opposes or beats me to it, I plan on merging this tomorrow. (EDIT: I may give it another pass, but doubt that will bring up anything).

@seberg
Copy link
Member

seberg commented Sep 9, 2019

Thanks everyone, lets put this in. I guess we can officially announce that we will merge it if there is no more discussion/concerns.

@seberg seberg force-pushed the nep-0032 branch 2 times, most recently from f1bf6fe to e72b4de Compare September 9, 2019 19:30
@seberg
Copy link
Member

seberg commented Sep 9, 2019

Renamed to NEP 31 and squashed. Will wait for CI and look at the rendered document and then merge.

@seberg seberg changed the title NEP 32: Remove the financial functions from NumPy NEP 31: Remove the financial functions from NumPy Sep 9, 2019
@rgommers
Copy link
Member

rgommers commented Sep 9, 2019

Please do not rename to NEP 31. This is and should remain 32 to avoid confusion.

@seberg
Copy link
Member

seberg commented Sep 9, 2019

@rgommers, hmmm, it was something I was not quite sure about. But the question is whether in general we should be OK with holes in nep numbering? You are right that it could be confusing to the discussion of the "unumpy for dispatching" proposal right now specifically.

@seberg
Copy link
Member

seberg commented Sep 9, 2019

It seems like PEPs are not even numbered in a regular fashion making the number just an identifier (I suppose there are probably some patterns). Should we basically adopt that?

@WarrenWeckesser
Copy link
Member Author

Thank Sebastian. I guess I shouldn't have explicitly referred to "NEP 32" in the mailing list announcement. Maybe we should modify NEP 0 to not recommend assigning a number when a NEP pull request is created. Perhaps initially give it a symbolic or mnemonic name (e.g. NEP-finfuncs or something like that), and defer assigning a number until the PR is merged.

@WarrenWeckesser
Copy link
Member Author

... or just do what we've been doing, and don't worry about maintaining consecutive order of the NEPs. So, as @rgommers, says, this NEP "is and should remain 32 to avoid confusion."

@stefanv
Copy link
Contributor

stefanv commented Sep 9, 2019

The original idea was that all NEPs are merged, and that the status flag would be changed depending on whether we decide to implement or reject the proposal. With this workflow, numbering on creation makes sense.

Ralf recently suggested that we perhaps wait a while before merging NEPs to make sure they are at least at a reasonable state of development. In that scenario, NEP numbers should be assigned only once we decide to merge a NEP.

Currently, NEP0 describes the first workflow, so we should come to a decision on which to use and modify the guidelines accordingly.

@WarrenWeckesser
Copy link
Member Author

While working on this NEP, I wrote down some thoughts on the question of when a NEP PR should be merged, so I have some suggestions that might clarify NEP 0. Let's continue the discussion of the process of merging a NEP elsewhere. Should it be a separate issue on github, or on the mailing list?

@rgommers
Copy link
Member

rgommers commented Sep 9, 2019

Numbering on creation seems to work. It also helps with associating mailing list discussions back to a PR. Minor holes (like merging 32 before 31) is not a problem. If there's another concern about how to pick numbers or something, let's just add a wiki page that people can edit to "reserve" a number or some such solution.

In that scenario, NEP numbers should be assigned only once we decide to merge a NEP.

I'd be -1 on that, since "NEP xx" is a very useful way to reference a proposal.

@seberg
Copy link
Member

seberg commented Sep 9, 2019

Well, I honestly do not mind much either way. I was thinking it can happen that we end up with an actual jump in the numbering, and it was a bit knee jerk to think that that should be avoided – even at the cost of conflicts – by just fixing/setting the numbering when the initial merge occurs.

@WarrenWeckesser WarrenWeckesser changed the title NEP 31: Remove the financial functions from NumPy NEP 32: Remove the financial functions from NumPy Sep 9, 2019
@WarrenWeckesser
Copy link
Member Author

I pushed an update to restore the number to 32, rename the file to include a few words that summarize the NEP, and squash the commits.

@seberg
Copy link
Member

seberg commented Sep 9, 2019

Thanks all, lets put this in and see if we can soon start removing these :).

@seberg seberg merged commit b42c2e3 into numpy:master Sep 9, 2019
@WarrenWeckesser WarrenWeckesser deleted the nep-0032 branch September 10, 2019 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants