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

pyOpenSci Initial Review Cleanup #144

Closed
JacksonBurns opened this issue Jul 12, 2023 · 2 comments · Fixed by #147
Closed

pyOpenSci Initial Review Cleanup #144

JacksonBurns opened this issue Jul 12, 2023 · 2 comments · Fixed by #147
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@JacksonBurns
Copy link
Owner

Feedback from Initial Submission to pyOpenSci

In the meanwhile I have a first question for you, if you don't mind.
While the statistical side of the package is clear and answers to a well known need of the scikit-learn user community, I would like to read a bit more about the package connections with chemical engineering. I guess your scientific application was the motivation for this development in the first place and in the documentation the two aspects look quite unrelated. Perhaps a small pedagogical section about chemical applications in the documentation will also be helpful? Thanks!

Originally posted by @cmarmo in pyOpenSci/software-submission#120 (comment)

We need to clarify the applications of astartes to chemical engineering in particular rather than just the importance in terms of statistics and ML, broadly speaking. As mentioned by @kspieks offline there is some content in the JOSS paper that could be pulled from to flesh this out.

I would suggest that we make a separate markdown document, like the 'sklearn to astartes' one and reference it in the main README.

@JacksonBurns JacksonBurns added the documentation Improvements or additions to documentation label Jul 12, 2023
@JacksonBurns JacksonBurns changed the title Clarify Domain Applications in Documentation pyOpenSci Initial Review Cleanup Jul 14, 2023
@JacksonBurns
Copy link
Owner Author

Additional Comments:

(see here for full explanations)

References

Many references in the README are paywalled, try and find open-access versions. Some references are also missing, include them. Also make sure that the References column of the Included Sampling Algorithms table clarifies what the link actually is a reference to (documentation, a paper, etc.).

Formatting Fixes

External links with backquotes are breaking in the Sphinx rendered documentation.

Documentation

Add further examples of some of the backend, and how users could leverage other functionality implemented in astartes besides just train_(val_)test_split.

Version Attribute

Specify a __version__ attribute, as well as directions for how to retrieve it. Can use this to keep compatibility with pyproject.toml:

[project]
name = "astartes"
dynamic = ["version"]

[tool.setuptools.dynamic]
version = {attr = "package.__version__"}

Which is compatible with both from importlib.metadata import version and astartes.__version__ approaches.

@JacksonBurns
Copy link
Owner Author

All of this is done in #147 except for fixing the references and some documentation for the internal functions.

kspieks added a commit that referenced this issue Jul 17, 2023
Resolves #144 - addressing comments and feedback from the initial
pyOpenSci editor review.
JacksonBurns added a commit that referenced this issue Jul 17, 2023
The previous patches for pyOpenSci (#144) introduced some issues in the
GitHub actions that we only saw once pushed to main - this PR fixes
them. This also pushes the version to 1.1.2 for the reviewers to
pyOpenSci.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants