# Modernize python in meas_base and meas_algorithms

XMLWordPrintable

#### Details

• Type: Improvement
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
0.25
• Sprint:
AP S18-5
• Team:

#### Description

Modernize how ndarray is called, remove python 2 support and automatic pep8 checking

#### Activity

Hide
Russell Owen added a comment -

Lauren MacArthur do you have time to look at this? It's a set of small changes on two packages. It may be a bit easier to review them commit by commit (since so much of it is automated and trivial), but please do it however you like.

Show
Russell Owen added a comment - Lauren MacArthur do you have time to look at this? It's a set of small changes on two packages. It may be a bit easier to review them commit by commit (since so much of it is automated and trivial), but please do it however you like.
Hide
Lauren MacArthur added a comment -

I'm happy to review, but I know all of nothing about pybind11 and proper "modern" usage of ndarray and I have yet to dip a toe into the world of "Travis".  Could you either point me to LSST documentation and/or coding standards that will allow me to asses the changes (or, perhaps more efficiently, point this to a reviewer who does know what is required!)?

Show
Lauren MacArthur added a comment - I'm happy to review, but I know all of nothing about pybind11 and proper "modern" usage of ndarray and I have yet to dip a toe into the world of "Travis".  Could you either point me to LSST documentation and/or coding standards that will allow me to asses the changes (or, perhaps more efficiently, point this to a reviewer who does know what is required!)?
Hide
Russell Owen added a comment -

Modernizing the use of ndarray in pybind11 wrappers is trivial:

1) Remove #include "numpy/arrayobject.h" (and any other numpy header files)
2) Remove the following:

  if (_import_array() < 0) {  PyErr_SetString(PyExc_ImportError, "numpy.core.multiarray failed to import");  return nullptr;  } 

I don't know where it's written down (Jim Bosch might know). However, it works, it makes warnings about using an outdated numpy API go away, and Jenkins is happy. I've done it to many packages now.

For travis, we have a standard .travis.yml file we add to a repo. It is described here:
Fortunately it needs to changes per repo.

The other step in enabling automatic pep8 checking with flake8 is to add a setup.cfg file. That one needs a few special exclusions if the package contains tasks (as meas_algorithms does), since tasks have those Doxygen ## blocks that flake8 hates. I'm not sure where the official docs are.

Show
Russell Owen added a comment - Modernizing the use of ndarray in pybind11 wrappers is trivial: 1) Remove #include "numpy/arrayobject.h" (and any other numpy header files) 2) Remove the following: if (_import_array() < 0) { PyErr_SetString(PyExc_ImportError, "numpy.core.multiarray failed to import"); return nullptr; } I don't know where it's written down ( Jim Bosch might know). However, it works, it makes warnings about using an outdated numpy API go away, and Jenkins is happy. I've done it to many packages now. For travis, we have a standard .travis.yml file we add to a repo. It is described here: https://developer.lsst.io/stack/adding-a-new-package.html?highlight=travis#configuring-github-repositories Fortunately it needs to changes per repo. The other step in enabling automatic pep8 checking with flake8 is to add a setup.cfg file. That one needs a few special exclusions if the package contains tasks (as meas_algorithms does), since tasks have those Doxygen ## blocks that flake8 hates. I'm not sure where the official docs are.
Hide
Jim Bosch added a comment -

Modernizing ndarray usage is not documented per se, but modern ndarray usage is:

https://developer.lsst.io/pybind11/how-to.html#ndarray

Show
Jim Bosch added a comment - Modernizing ndarray usage is not documented per se, but modern ndarray usage is: https://developer.lsst.io/pybind11/how-to.html#ndarray
Hide
John Parejko added a comment -

See comments on the PR. Thanks for doing this: you cleaned up more things that I'd started to.

Show
John Parejko added a comment - See comments on the PR. Thanks for doing this: you cleaned up more things that I'd started to.

#### People

Assignee:
Russell Owen
Reporter:
Russell Owen
Reviewers:
John Parejko
Watchers:
Jim Bosch, John Parejko, Lauren MacArthur, Russell Owen