-
-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New package version requirements #1351
New package version requirements #1351
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1351 +/- ##
==========================================
+ Coverage 99.64% 99.64% +<.01%
==========================================
Files 38 38
Lines 2805 2812 +7
==========================================
+ Hits 2795 2802 +7
Misses 10 10
Continue to review full report at Codecov.
|
I see this PR as less of an "experiment" and more of a philosophical decision on how to move forward with the requirements for Tax-Calculator. This PR positions Tax-Calculator to be "forward compatible" with numpy and pandas. That is, by specifying a version that is greater than or equal to some static version, we can take advantage of new versions of these packages, and only insist that the environment have versions that are no older than some set version. This is a very reasonable path, since we can take advantage of bug fixes and performance improvements in those packages. However, it is not without danger. The danger is that a new version of numpy or pandas will exhibit behavior that breaks the current Tax-Calculator. We have already seen this situation in the past. In issue #381, we had a situation where Tax-Calculator code worked with numpy 1.9, but not numpy 1.10, because there was a behavior change in numpy. I don't think there is one "right" answer here. If we make the conda recipe such that Tax-Calculator packages are "forward compatible" with numpy and pandas, a user's environment will likely be modified less than if we insist on some set version. However, we increase the risk that, over time, the Tax-Calculator code will actually be incompatible with some future version of numpy or pandas (or some other package) and the package will still successfully install and run. This can lead to issues like #381 or perhaps more nefarious situations where the code does not give an error but executes incorrectly (an admittedly unlikely scenario). |
Pull request #1351 has grown out of an assignment given to me several months ago by @MattHJensen. Matt asked me to develop user (not developer) documentation and a way to use Tax-Calculator that requires no Python programming knowledge. I told him I could write the HTML documentation, but that others with more web programming experience would have provide a CSS if he wanted the user documentation to look fancy. When giving me that assignment, Matt recommended a book to guide my effort: Karl Fogel, Producing Open Source Software: How to Run a Successful Free Software Project, Second Edition. Even though I have read only through page 36 of Fogel's book, it is clearly an excellent document with many useful observations. Before explaining the thinking behind pull request #1351, I want to quote extensively from page 11 of Fogel's book:
One of my take-aways from this passage is that the getting-started experience for new users should be as simple as possible and, as Fogel says, supply comfort and reassure users. So, in order to get an idea about what the new-user experience would be like when following the installation directions, I created a new account on my Mac that had no Anaconda or Tax-Calculator and followed the installation directions. One thing I found is that Continuum Analytics offers Anaconda distributions for Python 2.7 and for Python 3.6, but there is no taxcalc package for Python 3.6 (see issue #1339 for more details). So I followed the instruction at the Continuum Analytics download page and I had the Anaconda2 (version 4.3.1) Python 2.7 distribution installed in no time as all. Everything worked smoothly. Then I went to the next step and ran the
The first thought new users will probably have is "Why doesn't Tax-Calculator work with the version of Anaconda being offered by Continuum Analytics?" or "Why do I have to downgrade my Anaconda distribution in order to use Tax-Calculator?" This is not a good initial impression to leave with potential new users. The goal of this pull request is to eliminate this downgrading experience. @feenberg @PeterDSteinberg @brittainhard @talumbau |
Details of what was done in the development of pull request #1351I followed Continuum Analytics instructions about how to uninstall my old Anaconda2 (version 2.3.0) and then installed Anaconda2 (version 4.3.1). I did this because I could not figure out how to use the Then I edited the However, I noticed two sets of warnings that were printed to the screen that I had never seen before: one set of warnings occurred when using the It turns out that both sets of warnings are caused by improvements in numpy warning logic that are in numpy 1.11.3, but not in numpy 1.11.2. This pull request fixes the I also learned the the conda-build package is not part of the core Anaconda distribution, so I added code to the And finally, my new appreciation for the numpy.seterr method has led me to strengthen the unit tests by converting all numpy warnings into errors so they can be detected in the tests. This is done in I have experienced just one flaw with Anaconda2 version 4.3.1: the pylint utility does not work correctly without an additional
@MattHJensen @feenberg @PeterDSteinberg @brittainhard @talumbau |
I see the advantage of allowing most users to work with taxcalc without needing to create a conda environment and without downgrading their other packages, but I'm still worried about the question I asked over at #1350:
I think my problem is ignorance, because other projects also specify dependencies this way. For example, a dependency for pandas is "NumPy: 1.7.1 or higher" |
@MattHJensen said about pull request #1351:
That's a good question to pose. Here is what I think the answer is. There are two ways to guard against the problem Matt identified above.
If we do these two things (especially the second thing), any new version of numpy, pandas, bokeh, or any other package, that breaks Tax-Calculator will immediately be exposed. This is the way I understand the situation. Have I missed something? @feenberg @Amy-Xu @andersonfrailey @GoFroggyRun @codykallen @zrisher |
Thanks for the explanation. My takeaway is that a dependency's update might break taxcalc, but we'll catch it very quickly if it does. That seems like an ok state of things for now, and we can always reconsider if it causes too many problems. |
@MattHJensen said about pull request #1351:
Yes, I think that is a reasonable way to think about it. Shall we proceed and merge pull request #1351? |
+1 |
This pull request started out as an experiment to see how the tests run on GitHub when using less restrictive package versions in the
environment.yml
andconda.recipe/meta.yaml
files.See subsequent comments in the #1351 conversion for details on how this pull request has evolved.