Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

[BUGFIX]Fix chdir Error when invoked from Makefile #1193

Merged
merged 1 commit into from
Apr 2, 2020

Conversation

chenw23
Copy link
Member

@chenw23 chenw23 commented Apr 2, 2020

Description

This is a bug fix, rather than an enhancement.
Currently this file is invoked smoothly from CI, because CI passes the full path as the argument to this python script. However, when invoked from Makefile, using the command make docs, only file name is passed to this file, resulting in the input_dir variable to be empty string, so that the os.chdir() will fail.

More detailed explanation:
When we use make docs, the L60 of Makefile will be executed, and then L86 of Makefile will be executed. See in L89, variable BASENAME will be the file name and variable DIR will be the directory. In L96, only BASENAME is passed to MD2IPYNB as an argument. So there will be error in the python file.
With this fix, the python script doesn't need to change directory explicitly because in L92 of the Makefile, the directory is changed already. So this fix will work fine.

This pull request is compatible with the existing CI, and can make Makefile work fine again.

Checklist

Essentials

  • PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

cc @dmlc/gluon-nlp-team

@chenw23 chenw23 requested a review from a team as a code owner April 2, 2020 12:02
@codecov
Copy link

codecov bot commented Apr 2, 2020

Codecov Report

Merging #1193 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1193   +/-   ##
=======================================
  Coverage   87.42%   87.42%           
=======================================
  Files          81       81           
  Lines        7345     7345           
=======================================
  Hits         6421     6421           
  Misses        924      924

@mli
Copy link
Member

mli commented Apr 2, 2020

Job PR-1193/1 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1193/1/index.html

Copy link
Contributor

@leezu leezu left a comment

Choose a reason for hiding this comment

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

Thank you

@leezu leezu merged commit f092e60 into dmlc:master Apr 2, 2020
chenw23 pushed a commit to chenw23/gluon-nlp that referenced this pull request Apr 15, 2020
Now adopts the great new enhancements from dmlc#1191 , dmlc#1192 , dmlc#1193 , dmlc#1194 , dmlc#1195 , dmlc#1196 , dmlc#1197 , dmlc#1198 and dmlc#1120 .
The pull requests above fixed various errors in the previous make docs command and added new switch options. Now the commands mentioned in this documentation are strictly error-free.
chenw23 pushed a commit to chenw23/gluon-nlp that referenced this pull request Apr 15, 2020
Now adopts the great new enhancements from dmlc#1191 , dmlc#1192 , dmlc#1193 , dmlc#1195 , dmlc#1196 , dmlc#1197 , dmlc#1198 and dmlc#1200 .
The pull requests above fixed various errors in the previous make docs command and added new switch options. Now the commands mentioned in this documentation are strictly error-free.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants