Skip to content

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

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

Allstreamer
Copy link
Collaborator

@Allstreamer Allstreamer commented Aug 8, 2022

  • 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

Implementaition of what was mentioned in

- 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
@John15321
Copy link
Contributor

Please change target to the develop branch

@Allstreamer Allstreamer changed the base branch from main to develop August 8, 2022 20:17
@Allstreamer
Copy link
Collaborator Author

Solved the merge conflicts and retargeted to develop

- Solved clippy suggestions
- Ran cargo fmt
@Allstreamer
Copy link
Collaborator Author

Fixed Formating & Linting

@John15321
Copy link
Contributor

When writing the name pypi with capital letters make sure its "PyPI" not Pypi or PyPi that also includes structs

@John15321
Copy link
Contributor

Please add docstrings to all structs, methodsz functions etc

@John15321
Copy link
Contributor

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.
I know it may be complicated but that's why we don't have a lot of written code rn. I have to research mock modules etc we want to use so try ur best for now

@John15321
Copy link
Contributor

John15321 commented Aug 9, 2022

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

@John15321 John15321 added documentation Improvements or additions to documentation new feature New feature or request labels Aug 9, 2022
- Added Docstrings
- Renamed PypiData to PyPIData
- Added unit tests
@Allstreamer
Copy link
Collaborator Author

Should now be mergable

- Renamed check_pytorch_download_v1 to check_numpy_download_name_v1
@Allstreamer
Copy link
Collaborator Author

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
@Kiwifuit
Copy link

Kiwifuit commented Aug 9, 2022

You could have your PyPIPackageInfo look like this since we wont need most of the info that the JSON API gives:

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,
}

@Allstreamer
Copy link
Collaborator Author

Not every package uses semver (see package "AWRS" which has the version 2019.8.14.1152)

@Allstreamer
Copy link
Collaborator Author

I'll probably rewrite it that way but first i need to find out everything that rust-pip could need

@John15321
Copy link
Contributor

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

@Allstreamer
Copy link
Collaborator Author

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:
>=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*

A way of managing package versions and release information,
Since not every release that's listed in the api has information.

and so on!

So i'll be crafting away at this pull request for a while!

@Allstreamer
Copy link
Collaborator Author

#[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

@John15321
Copy link
Contributor

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

@John15321 John15321 self-requested a review August 9, 2022 15:05
@Allstreamer Allstreamer marked this pull request as draft August 9, 2022 15:06
@Allstreamer
Copy link
Collaborator Author

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

@Allstreamer
Copy link
Collaborator Author

Allstreamer commented Aug 9, 2022

I've been looking into how pip manages versions and it looks like they have two systems:

  • Their own version system in this format: int, Tuple[int, ...], PrePostDevType, PrePostDevType, PrePostDevType, LocalType
  • And a legacy system which allows abitrary strings:
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 Err when reciving an abitrary string

@John15321
Copy link
Contributor

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 {

Choose a reason for hiding this comment

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

Suggested change
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.

Copy link
Collaborator Author

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;

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.

@DevChaudhary78 DevChaudhary78 marked this pull request as ready for review May 14, 2024 17:11
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 new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants