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

feat(backtesting) include button to export csv #86

Merged

Conversation

panapol-p
Copy link
Contributor

What did not work before?

Include button to export CSV with orders in chart page #53

What have you changed and why?

add button for download trading history
add new api to get trading history by pair

This PR has:
  • Been doubled checked by the author
  • Tested for newly introduced bugs
  • Included documentation

image

@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2021

Codecov Report

Merging #86 (ac7c5d4) into main (3a4ef9d) will increase coverage by 0.08%.
The diff coverage is 21.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   50.07%   50.15%   +0.08%     
==========================================
  Files          14       15       +1     
  Lines        1909     2199     +290     
==========================================
+ Hits          956     1103     +147     
- Misses        895     1033     +138     
- Partials       58       63       +5     
Impacted Files Coverage Δ
plot/chart.go 50.68% <21.73%> (ø)

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 3a4ef9d...ac7c5d4. Read the comment docs.

@rodrigo-brito
Copy link
Owner

rodrigo-brito commented Jan 1, 2022

Hi @panapol-p, thank you.
It looks great.

Copy link
Owner

@rodrigo-brito rodrigo-brito left a comment

Choose a reason for hiding this comment

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

Just a couple of comments.

exchange/paperwallet.go Outdated Show resolved Hide resolved
plot/chart.go Outdated Show resolved Hide resolved
plot/chart.go Outdated Show resolved Hide resolved
Copy link
Owner

@rodrigo-brito rodrigo-brito left a comment

Choose a reason for hiding this comment

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

Hi @panapol-p, great job! 👏🏻👏🏻👏🏻
Just a small comment.

plot/assets/chart.html Outdated Show resolved Hide resolved
@panapol-p
Copy link
Contributor Author

Hi, I have some questions.
The codecov/patch is calculated from the subfolder too, right?

@rodrigo-brito
Copy link
Owner

Hi @panapol-p, don't worry about the coverage. I try to improve it later. Your code is well tested.

@rodrigo-brito rodrigo-brito merged commit 3cec963 into rodrigo-brito:main Jan 22, 2022
@rodrigo-brito
Copy link
Owner

Thanks @panapol-p, great job!

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.

None yet

3 participants