search

tboothman / imdbphp

PHP library for retrieving film and tv information from IMDb
251 stars 84 forks source link

Added tvSeriesByYearCount() method #243

Closed duck7000 closed 1 year ago

duck7000 commented 3 years ago

It returns int based on how many valid years found. So for a tv series with 2002, 2003, 2004, 2005 it returns int 4

Thomasdouscha commented 3 years ago

I am impatiently waiting for this code :)

jreklund commented 3 years ago

Hi, can you give me a tv-show that does this. So I can test it out fully.

duck7000 commented 3 years ago

sure, this one: https://www.imdb.com/title/tt0388629/

duck7000 commented 3 years ago

I thought about combining this with seasons but the output will be different as it aren't seasons but years?

jreklund commented 3 years ago

Hmm, if we consider this to be a different value "not a season" then it should not be named as such e.g. "how many years this show have been running"-name. That would be more correct I guess, as episodes are just episodes no matter when/how it aired.

duck7000 commented 3 years ago

Correct then it wouldn't be "season"by year as it aren't seasons. Good point, i'll change the method name.

I have no clue what others think about this? @tboothman ?

And i'm not using this method myself as i don't jet need it (my tv series are all season based so far) but that might change in the future.

duck7000 commented 3 years ago

how about this as method name: tvSeriesYearCount or tvSeriesEpisodesByYear or tvSeriesByYear

It is very hard to come up with something useful

jreklund commented 3 years ago

@duck7000 Naming convention are the hardest part of programming lol. That's why I didn't give you an example, didn't come up with one myself.

duck7000 commented 3 years ago

Is this ready to get a final review jet?

duck7000 commented 3 years ago

Closing this one, No more response, I don't understand why

duck7000 commented 2 years ago

On request from @Thomasdouscha i will reopen this pr

duck7000 commented 2 years ago

@Thomasdouscha Can you test this method to make sure it all works well? Try to find some more examples to test it, i only found one.

I don't get any response from @jreklund but if you test and it works at least you can use it in your program i guess

Thomasdouscha commented 2 years ago

Yes i can but again i need the file.

duck7000 commented 2 years ago

Okay here it is, it is up to date with the master from imdbphp but includes this method Method name is tvSeriesByYearCount() Title.tvSeriesByYearCount.zip

Thomasdouscha commented 2 years ago

image

It works but there is one mistake i explain you :

image

as you see 999.episodes highest limit. It should be 9999 minimum. Because if the episodes are based year that means there are too much episodes. I saw even 35000 episodes.

duck7000 commented 2 years ago

It may be that you are missing the point of this method?

It just returns the number of years if the tv series is year based. It does exactly the same as seasons() but instead count how many years that series runs So in case of one piece it will return int 24, that is all and has nothing to do with episodes.

If you mean that there is a limit on how many episodes can be returned from episodes() method than that is a bug inside that method, not in this method.

duck7000 commented 2 years ago

It might also be that your database field has this limit? I can't think of something that would limit the amount of episodes.

Thomasdouscha commented 2 years ago

No. 10 digit even is possible. varchar(10). it is up to here. Maybe not from directly your code.

Thomasdouscha commented 2 years ago

Database ok. I will check code and has not issue. It is up to yourside

duck7000 commented 2 years ago

I still think you are missing the point of this specific method that you not seem to use in your example screenshots?

This specific method just counts the amount of years just like seasons() does for season based tv series. It can't give output like your screenshots!

Witch method did you use to make above screenshots? And what do you mean by "It is up to yourside" I did not make method episodes() so don't blame me for something i didn't do

Thomasdouscha commented 2 years ago

If episodes get just maximum 999 episodes, Tvseriesbyyearcount for what? As i told you tvseries which year based has too much episodes. 999 well not enough. Tomorrow i will look at again and i will add my code here. Again we can concidere it

duck7000 commented 2 years ago

I thought that this method is what you impatiently are waiting for? Now you say "for what"

Maybe we are not on the same page and think in different directions here?

What did you expect from this method? As i explained before it only returns the number of years just like seasons() does for season based series.

I think that you use episodes() and that there in your perspective is something wrong with it, hence you mentioned episodes can't be more than 999 You have to be more specific in what you want and expect, i'm lost at this point as we clearly not talking about the same thing.

Thomasdouscha commented 2 years ago

yes you are right. I am ill thatswy i couldnot test your code. Sorry @duck7000.

duck7000 commented 1 year ago

@Thomasdouscha Is this still something to discuss about? If not i will close it because i don't think anyone would use it

Thomasdouscha commented 1 year ago

I will use it. Because some tv series episodes based years. Not less . What was the issue about it. If you revise it i can test again.

duck7000 commented 1 year ago

well last i checked it does what it supposed to do, returning the number of years a tv series run. So yes you can use it but it would be nice if this get merged as well

Thomasdouscha commented 1 year ago

So why did not they merge it?

duck7000 @.***>, 10 Eki 2022 Pzt, 15:50 tarihinde şunu yazdı:

well last i checked it does what it supposed to do, returning the number of years a tv series run. So yes you can use it but it would be nice if this get merged as well

— Reply to this email directly, view it on GitHub https://github.com/tboothman/imdbphp/pull/243#issuecomment-1273267449, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKPFI3FBUIW7XTLRM2AO3T3WCQGIJANCNFSM5AEKZMZQ . You are receiving this because you were mentioned.Message ID: @.***>

Thomasdouscha commented 1 year ago

Please let you merge it after clarify ok.

duck7000 commented 1 year ago

Well that is not up to me, @tboothman only could merge it so we will have to wait and see. I don't understand either why this takes so long, some PR are open for more than a year. Sure it takes time to review but not that long i guess.

Only other option is for you to fork this repo and make your own version.

I did raised a issue that I'm concerned that PR requests take a very long time here, only @jreklund responded that he has no time or interest in reviewing PR anymore, and sure that is fine and clear, but means it is only up to @tboothman to act on PR requests