Skip to content
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

[Docs] Replace markdownlint with mdformat and configure myst-parser #493

Closed
wants to merge 10 commits into from

Conversation

RunningLeon
Copy link
Collaborator

Motivation

Following:
Replace markdownlint: open-mmlab/mmcv#1936
Configure Myst-parser to parse anchor tag: open-mmlab/mmocr#1012

Modification

md docs

BC-breaking (Optional)

None

@RunningLeon RunningLeon requested a review from lvhan028 May 18, 2022 03:35
@RunningLeon RunningLeon changed the title [Docs] Replace markdownlint with mdformat for avoiding installing ruby [Docs] Replace markdownlint with mdformat and configure myst-parser May 18, 2022
@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

Merging #493 (9acd0ea) into master (a228f95) will decrease coverage by 0.23%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master     #493      +/-   ##
==========================================
- Coverage   57.38%   57.14%   -0.24%     
==========================================
  Files         259      259              
  Lines        8574     8574              
  Branches     1281     1281              
==========================================
- Hits         4920     4900      -20     
- Misses       3301     3322      +21     
+ Partials      353      352       -1     
Flag Coverage Δ
unittests 57.14% <50.00%> (-0.24%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmdeploy/apis/inference.py 40.00% <ø> (ø)
mmdeploy/apis/pytorch2onnx.py 30.30% <ø> (ø)
mmdeploy/apis/visualize.py 28.00% <ø> (ø)
mmdeploy/backend/tensorrt/onnx2tensorrt.py 0.00% <ø> (ø)
...oy/codebase/mmdet/core/post_processing/bbox_nms.py 46.61% <0.00%> (-16.95%) ⬇️
mmdeploy/core/optimizers/function_marker.py 69.91% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a228f95...9acd0ea. Read the comment docs.

@lvhan028
Copy link
Collaborator

Let's deal with it when dev-0.5.0 is ready. Otherwise, lots of conflicts will drive us mad

@lvhan028
Copy link
Collaborator

'fcos_r50_caffe_fpn_gn-head_1x_coco.py')
>>> model_checkpoint = ('checkpoints/'
'fcos_r50_caffe_fpn_gn-head_1x_coco-821213aa.pth')
>>> deploy_cfg = 'configs/mmdet/detection/' \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this line broken by pre-commit automatically? '\' makes API docs not pretty in readthedocs. We used to fix it in PR #495

@@ -154,7 +154,7 @@ def impl(ys, prefix, level):
if ys not in visit:
visit.add(ys)
root = ctx.names[ctx.index]
name = '.'.join(str(x) for x in (root, *prefix))
name = '/'.join(str(x) for x in (root, *prefix))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it has been fixed in another PR. May rebase master

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. Before there are a lot of conflicts. To make it clean, I reverted some commits. Maybe something wrong when revert the merge commit. rebase to master not work in this case.

@@ -201,7 +201,7 @@ def multiclass_nms__default(ctx,
"""
deploy_cfg = ctx.cfg
batch_size = boxes.size(0)
if not is_dynamic_batch(deploy_cfg) and batch_size == 1:
if not is_dynamic_batch(deploy_cfg) and batch_size != 1:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it has been fixed in another PR. May rebase master

| ONNX Runtime | TensorRT | ppl.nn | ncnn | OpenVINO | more |
| ------------ | -------- | ------ | ---- | -------- | ------------------------------------------------- |
| ✔️ | ✔️ | ✔️ | ✔️ | ✔️ | [benchmark](docs/zh_cn/03-benchmark/benchmark.md) |
| ✔️ | ✔️ | ✔️ | ✔️ | ✔️ | [benchmark](docs/zh_cn/03-benchmark/benchmark.md) |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi, @zhouzaida mdformat adjusts this table by mistake.

'fcos_r50_caffe_fpn_gn-head_1x_coco.py')
>>> deploy_cfg = ('configs/mmdet/detection/'
'detection_onnxruntime_dynamic.py')
>>> model_cfg = 'mmdetection/configs/fcos/' \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this line broken by pre-commit automatically? '' makes API docs not pretty in readthedocs. We used to fix it in PR #495

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll update it. maybe there's a mistake when revert merge commit.

@lvhan028
Copy link
Collaborator

How about we drop this PR and apply "markdownlint: open-mmlab/mmcv#1936" and "open-mmlab/mmocr#1012" to the latest mmdeploy directly?

@RunningLeon
Copy link
Collaborator Author

How about we drop this PR and apply "markdownlint: open-mmlab/mmcv#1936" and "open-mmlab/mmocr#1012" to the latest mmdeploy directly?

OK

@RunningLeon
Copy link
Collaborator Author

move to #610

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants