-
Notifications
You must be signed in to change notification settings - Fork 10
Implemented Warehouse Pypi API Call #16
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
base: develop
Are you sure you want to change the base?
Conversation
- Added log 0.4 Crate - Added serde 1.0 Crate - Added serde_json 1.0 Crate - Created pypi.rs - Created request_package_info function - Created PypiData struct - Added request_package_info example to main.rs
Please change target to the develop branch |
Solved the merge conflicts and retargeted to develop |
- Solved clippy suggestions - Ran cargo fmt
Fixed Formating & Linting |
When writing the name pypi with capital letters make sure its "PyPI" not Pypi or PyPi that also includes structs |
Please add docstrings to all structs, methodsz functions etc |
Please add tests. Both usage unit tests in the example section of the docstrings and negative cases in a test module and some integration tests to check if the cli works. |
Additionally i don't see any issues connected to this PR that explain what's being done here. So please create one and link it |
- Added Docstrings - Renamed PypiData to PyPIData - Added unit tests
Should now be mergable |
- Renamed check_pytorch_download_v1 to check_numpy_download_name_v1
Had a name mixed up where i was getting info for numpy but the test was named pytorch |
- Added Negative test case (Haven't added others for .get() calls since those will be replaced soon with more specific structs)
- Fixed some doc strings
- Created PyPIPackageInfo & PyPIPackageDownloadInfo Structs - Updated unit-tests, examples & doc examples
You could have your struct PyPIPackageInfo {
version: String, // Or a tuple of MAJOR, MINOR, PATCH assuming that every python package uses semver
digest: String,
url: String,
python_version: String, // Again, semver tuple can be used here
size: u8, // Can be helpful in progress bars
filename: String,
} |
Not every package uses semver (see package "AWRS" which has the version 2019.8.14.1152) |
I'll probably rewrite it that way but first i need to find out everything that rust-pip could need |
This is looking better and better. Don't worry we are not in a rush. It's not like anyone's gonna rewrite pip in rust before us. And personally due to multiple private reasons i cannot do us too much on this project so chill |
Yeah, don't worry too much! I'm still a long ways away from being happy with the code in this pull request! Still need a way to calculate python version requirements from strings such as: A way of managing package versions and release information, and so on! So i'll be crafting away at this pull request for a while! |
#[derive(Debug, Serialize, Deserialize, PartialEq)]
pub struct PyPIPackageRelease {
version: String,
filename: String,
md5_digest: String,
required_python: String,
size: u32,
url: String,
}
#[derive(Debug, Serialize, Deserialize, PartialEq)]
pub struct PyPIPackageData {
name: String,
stable_version: String,
releases: Vec<PyPIPackageRelease>
} Something like this but with the versions replaced with some kind of struct is what i'm thinking of @Kiwifuit |
I remember finding crates that solve server for you maybe you can find something. Also please mark this PR as draft if that's the case |
Sadly PyPi allows arbitrary strings and not just semver |
I've been looking into how pip manages versions and it looks like they have two systems:
class LegacyVersion(_BaseVersion):
def __init__(self, version: str) -> None:
self._version = str(version)
self._key = _legacy_cmpkey(self._version)
warnings.warn(
"Creating a LegacyVersion has been deprecated and will be "
"removed in the next major release",
DeprecationWarning,
) Since as it says above that the legacy system will be removed with the next release i think i should just implement their current version system and throw an |
Yeah in general i don't plan on supporting anything legacy. This is supposed to be bloatless pip for the future |
|
||
/// Download stats | ||
#[derive(Debug, Serialize, Deserialize, PartialEq)] | ||
pub struct PyPIPackageDownloadInfo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub struct PyPIPackageDownloadInfo { | |
pub struct PypiPackageDownloadInfo { |
The typical Rust convention is to keep acronyms with the first letter in uppercase, and the rest in lowercase. This would also need to be updated in the rest of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
John wanted this formatting and I think matching the Offical naming looks better.
src/pypi.rs
Outdated
/// | ||
/// # Example | ||
/// ``` | ||
/// use rust-pip::PyPI::request_package_info; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure this would error with invalid syntax. Have you tried running the doc tests? That should be cargo test --doc
.
Implementaition of what was mentioned in