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

Added the dunder str to repr.py and updated the file name #27

Merged
merged 12 commits into from
Jan 22, 2024

Conversation

isFakeAccount
Copy link
Contributor

  • Added the _str() in the repr.py
  • Updated the repr.py filename to str_and_repr.py
  • Also added the .idea in gitirnore for jetbrain Ides

Copy link
Owner

@trag1c trag1c left a comment

Choose a reason for hiding this comment

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

I think including !a would be nice too :)

@trag1c
Copy link
Owner

trag1c commented Jun 23, 2023

It should be made clear that !s is the default specifier and that f"{x!s}" is equivalent to just f"{x}" 👍

@isFakeAccount
Copy link
Contributor Author

crap, I forgot that it would add the fixed point file here

@trag1c
Copy link
Owner

trag1c commented Jun 23, 2023

crap, I forgot that it would add the fixed point file here

that's why you should use branches 💀💀💀 please revert

@isFakeAccount
Copy link
Contributor Author

Gone, Reduced to Atoms

@isFakeAccount
Copy link
Contributor Author

wtf

@isFakeAccount
Copy link
Contributor Author

#29 Linking issue to this PR

self.value = value

def __repr__(self) -> str:
"""Repr prints the python code to create this python object."""

Choose a reason for hiding this comment

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

I wouldn't state this as fact. Even the stdlib doesn't follow this. Also it technically doesn't "print" anything.

Instead, I'd say something like "Generally, __repr__ returns the code for creating the instance as a string".

Copy link
Owner

Choose a reason for hiding this comment

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

This! Alternatively "__repr__ should return [...]".

@@ -3,7 +3,7 @@ def __init__(self, *, value: str) -> None:
self.value = value

def __repr__(self) -> str:
"""Repr prints the python code to create this python object."""
"""repr returns string containing a printable representation of an object"""

Choose a reason for hiding this comment

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

strings are inherently printable?

maybe "Returns a string containing the code for creating the object"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

strings are inherently printable?

maybe "Returns a string containing the code for creating the object"?

Yes, that's what the official docs say. https://docs.python.org/3/library/functions.html#repr

Choose a reason for hiding this comment

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

I still feel like it's a poor description

Copy link
Contributor Author

@isFakeAccount isFakeAccount Jun 28, 2023

Choose a reason for hiding this comment

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

I disagree. Weird thing to nitpick on. Besides, is it guaranteed that repr will always yield that object?

For many types, this function makes an attempt to return a string that would yield an object with the same value when passed to eval()

The official docs do not say for all types.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even though it's not an absolute requirement I think that it should be indicated on the docstring.

@isFakeAccount
Copy link
Contributor Author

I rebased off the main branch which is why it looks like this

.gitignore Outdated
.idea/
Copy link
Contributor

Choose a reason for hiding this comment

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

No EOL at EOF

@@ -3,7 +3,7 @@ def __init__(self, *, value: str) -> None:
self.value = value

def __repr__(self) -> str:
"""Repr prints the python code to create this python object."""
"""repr returns string containing a printable representation of an object"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though it's not an absolute requirement I think that it should be indicated on the docstring.



if __name__ == "__main__":
my_class = MyCoolClass(value="© OpenAI ®")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too fond of this example string. Maybe something like ∫ f(x) dx would be nice as a nod to James's personal website.

#
# digit_regex = re.compile(r"\d")
# x = lambda y: y
# o = object()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why isn't the repr of these examples printed out?

.gitignore Outdated
.idea/

Copy link
Contributor

Choose a reason for hiding this comment

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

Now there's an extraneous newline. Not really a problem though.

- Added the __str_() in the repr.py
- Updated the repr.py filename to str_and_repr.py
Corrected Doc-String for repr based from PR feedback
Elaborated on when evaling a repr should give us identical object and also the cases when it doesn't
Made black not cry
- Edited __repr__ doc 
- Added EOL in gitignore
- Removed extra line from .gitignore
- Converted lambda to function since ruff was complaining
Removed docs, comments, and some code, so the file can fit on screen.
Removed args only limitation from init
@PiCake314
Copy link

oh yeah good stuf

Copy link

@PiCake314 PiCake314 left a comment

Choose a reason for hiding this comment

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

Good example. Easy to follow and makes sense.

Copy link

@Enderchief Enderchief left a comment

Choose a reason for hiding this comment

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

LGTM

@trag1c trag1c merged commit 013e378 into trag1c:main Jan 22, 2024
1 check passed
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.

7 participants