Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Rename np_compat to np_shape #15063

Merged
merged 8 commits into from
May 26, 2019
Merged

Conversation

reminisce
Copy link
Contributor

Description

This PR renames np_compat to np_shape for being more specific, since it only affects the shape inference logic. This needs to be merged to 1.5 release.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Copy link
Member

@eric-haibin-lin eric-haibin-lin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

python/mxnet/util.py Show resolved Hide resolved
@karan6181
Copy link
Contributor

@mxnet-label-bot add [Numpy, pr-awaiting-review]

@marcoabreu marcoabreu added Numpy pr-awaiting-review PR is waiting for code review labels May 24, 2019

def set_np_shape(active):
"""
Turns on/off NumPy shape semantics. This is turned off by default for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document what numpy shape semantics means.


def is_np_shape():
"""
Checks whether the NumPy shape semantics is currently turned on.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@reminisce reminisce force-pushed the rename_np_compat_to_np_shape branch from 6fe08c2 to ba992c2 Compare May 25, 2019 20:58
@szha szha merged commit 7edd69e into apache:master May 26, 2019
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* Change np_compat to np_shape

* Fix scala

* Fix pylint

* Add examples and fix documentation

* Fix doc

* More doc

* Rename np_compat to np_shape in test_operatory.py

* Rename in ndarray.cc
* \param prev returns the previous status before this set
* \return 0 when success, -1 when failure happens
*/
MXNET_DLL int MXSetIsNumpyCompatible(int is_np_comp, int* prev);
MXNET_DLL int MXSetIsNumpyShape(int is_np_shape, int* prev);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes how array format is expected when loading from disk. It would be good to document this on the function call. If NumpyShape is on, then Loading ndarray will fail. Just happened to me now. I think this is a side effect which should be clearly documented, or can we add additional arguments to Load with the different semantics?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Numpy pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants